Re: Review Request 33647: Only log processes in the slave cgroup on recovery.

2015-04-28 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33647/#review81905 --- Ship it! Ship It! - Timothy Chen On April 28, 2015, 11:48 p.m.,

Re: Review Request 33647: Only log processes in the slave cgroup on recovery.

2015-04-28 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33647/ --- (Updated April 28, 2015, 4:48 p.m.) Review request for mesos, Ben Mahler, Jie Y

Re: Review Request 33647: Only log processes in the slave cgroup on recovery.

2015-04-28 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33647/ --- (Updated April 28, 2015, 4:47 p.m.) Review request for mesos, Ben Mahler, Jie Y

Re: Review Request 33644: MemoryTestHelper: Changed to do memset before mlock.

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

Re: Review Request 33647: Only log processes in the slave cgroup on recovery.

2015-04-28 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33647/#review81903 --- Ship it! src/slave/slave.cpp

Review Request 33647: Only log processes in the slave cgroup on recovery.

2015-04-28 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33647/ --- Review request for mesos, Ben Mahler, Jie Yu, and Timothy Chen. Bugs: MESOS-266

Re: Review Request 33644: MemoryTestHelper: Changed to do memset before mlock.

2015-04-28 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33644/#review81899 --- Ship it! Ship It! - Jie Yu On April 28, 2015, 11 p.m., Chi Zhang

Review Request 33644: MemoryTestHelper: Changed to do memset before mlock.

2015-04-28 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33644/ --- Review request for mesos and Jie Yu. Bugs: mesos-2660 https://issues.apache

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-28 Thread Michael Park
> On April 22, 2015, 12:38 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, lines 2458-2459 > > > > > > These two fields are optional, `principal` doesn't have a default. Do > > we need to check it? Can a

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-28 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32150/ --- (Updated April 28, 2015, 10:43 p.m.) Review request for mesos, Alexander Ruklet

Re: Review Request 32108: Added manual make for readability training source code.

2015-04-28 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32108/ --- (Updated April 28, 2015, 3:36 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 32108: Added manual make for readability training source code

2015-04-28 Thread Bernd Mathiske
> On April 28, 2015, 12:15 a.m., Alexander Rukletsov wrote: > > Could you please add a period in summary? Also, don't we need license > > snippet at the top of each file? Both fixed. thx! - Bernd --- This is an automatically generated

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-28 Thread Michael Park
> On April 22, 2015, 12:38 p.m., Alexander Rukletsov wrote: > > src/tests/master_validation_tests.cpp, line 247 > > > > > > As I have mentioned earlier, how about a test where we `FrameworkInfo` > > doesn't have a pri

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-28 Thread Michael Park
> On April 22, 2015, 12:38 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, lines 2458-2459 > > > > > > These two fields are optional, `principal` doesn't have a default. Do > > we need to check it? Can a

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-28 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32150/ --- (Updated April 28, 2015, 10:21 p.m.) Review request for mesos, Alexander Ruklet

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

2015-04-28 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29889/#review81873 --- Dockerfile

Re: Review Request 33572: Add C++11 unrestricted union to the C++ style guide.

2015-04-28 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33572/#review81881 --- Ship it! Keeping `union` confined to library implementation sounds

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-28 Thread Michael Park
> On April 22, 2015, 12:38 p.m., Alexander Rukletsov wrote: > > src/master/validation.cpp, line 555 > > > > > > Let's leave a comment here that `resource::validate` not only checks > > for integrity of the `resources`

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-28 Thread Michael Park
> On April 22, 2015, 12:38 p.m., Alexander Rukletsov wrote: > > src/tests/master_validation_tests.cpp, line 328 > > > > > > Don't you get an unused variable warning? Hm, looks like I do. Not sure what happpened there.

Re: Review Request 32150: Enabled the master to handle reservation operations.

2015-04-28 Thread Michael Park
> On April 8, 2015, 10:38 p.m., Jie Yu wrote: > > src/master/validation.cpp, lines 581-584 > > > > > > Is this necessary, or will be captured by the resources contains check > > later? It probably would be checked by

Re: Review Request 33631: Fixed a bug in port mapping isolator which will cause SIGABRT during slave recovery.

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

Re: Review Request 33631: Fixed a bug in port mapping isolator which will cause SIGABRT during slave recovery.

2015-04-28 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33631/#review81848 --- Ship it! Ship It! - Ian Downes On April 28, 2015, 10:55 a.m., Ji

Re: Review Request 33631: Fixed a bug in port mapping isolator which will cause SIGABRT during slave recovery.

2015-04-28 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33631/#review81846 --- Ship it! Ship It! - Chi Zhang On April 28, 2015, 5:55 p.m., Jie

Review Request 33631: Fixed a bug in port mapping isolator which will cause SIGABRT during slave recovery.

2015-04-28 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33631/ --- Review request for mesos, Ben Mahler, Chi Zhang, and Ian Downes. Bugs: MESOS-26

Re: Review Request 33558: Add C++11 lambdas to the C++ style guide.

2015-04-28 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33558/ --- (Updated April 28, 2015, 5:33 p.m.) Review request for mesos, Ben Mahler, Joris

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

2015-04-28 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33295/#review81825 --- 3rdparty/libprocess/include/process/process.hpp

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

2015-04-28 Thread Michael Park
> On April 23, 2015, 5:20 p.m., Michael Park wrote: > > src/common/parse.hpp, line 46 > > > > > > nit: `s/template<>/template <>/` > > Alexander Rojas wrote: > I was checking and all the entries in this file have t

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

2015-04-28 Thread Alexander Rojas
> On April 23, 2015, 8:19 p.m., Joris Van Remoortere wrote: > > 3rdparty/libprocess/src/process.cpp, lines 2461-2463 > > > > > > For this synchronized block, and above: > > > > In order to ensure we don't end

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

2015-04-28 Thread Alexander Rojas
> On April 23, 2015, 7:20 p.m., Michael Park wrote: > > src/common/parse.hpp, line 46 > > > > > > nit: `s/template<>/template <>/` I was checking and all the entries in this file have the from `template<>` should I ch

Re: Review Request 32108: Added manual make for readability training source code

2015-04-28 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32108/#review81783 --- Could you please add a period in summary? Also, don't we need licens

Re: Review Request 32140: Enabled 'Resources' to handle 'Resource::ReservationInfo'.

2015-04-28 Thread Alexander Rukletsov
> On April 15, 2015, 5:36 p.m., Alexander Rukletsov wrote: > > src/common/resources.cpp, lines 455-464 > > > > > > Agree with Jie, I spent some time trying to understand what's going on > > here and what the implicati

Re: Review Request 32139: Added 'Resource::ReservationInfo' protobuf message.

2015-04-28 Thread Alexander Rukletsov
> On April 24, 2015, 8:19 a.m., Alexander Rukletsov wrote: > > include/mesos/mesos.proto, lines 397-398 > > > > > > While reviewing `Resources::isDynamicReservation()` from > > https://reviews.apache.org/r/32398/ I re