Re: Review Request 25270: Enable bridge network in Mesos

2014-09-15 Thread Derek Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25270/#review53312 --- Ship it! Ship It! - Derek Zhang On Sept. 11, 2014, 5:48 p.m.,

Build failed in Jenkins: mesos-reviewbot #1553

2014-09-15 Thread Apache Jenkins Server
See https://builds.apache.org/job/mesos-reviewbot/1553/ -- [...truncated 2033 lines...] make[9]: Entering directory `https://builds.apache.org/job/mesos-reviewbot/ws/mesos-0.21.0/_build/3rdparty/libprocess/3rdparty/protobuf-2.5.0' Making all in . make[10]:

Re: Review Request 25035: Fix for MESOS-1688

2014-09-15 Thread Timothy St. Clair
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25035/#review53343 --- src/master/hierarchical_allocator_process.hpp

Jenkins build is back to normal : mesos-reviewbot #1554

2014-09-15 Thread Apache Jenkins Server
See https://builds.apache.org/job/mesos-reviewbot/1554/

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/#review53352 --- Ship it! Looks like it is ready to go :)

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/#review53353 --- 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

2014-09-15 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review53355 --- docs/mesos-c++-style-guide.md

Re: Review Request 25623: MESOS-1794: allow CURL download into any FD

2014-09-15 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25623/#review53356 --- 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp

Re: Review Request 25035: Fix for MESOS-1688

2014-09-15 Thread Vinod Kone
On Sept. 15, 2014, 3:23 p.m., Timothy St. Clair wrote: src/master/hierarchical_allocator_process.hpp, line 837 https://reviews.apache.org/r/25035/diff/7/?file=688721#file688721line837 What happens in the case where all CPUs are taken but memory is available? It looks like it

Re: Review Request 25618: MESOS-1792: add os::shell with pluggable IO descriptors

2014-09-15 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25618/#review53360 --- Drive by question: what does this offer that can't already be done

Re: Review Request 25623: MESOS-1794: allow CURL download into any FD

2014-09-15 Thread Kamil Domanski
On Sept. 15, 2014, 7:15 p.m., Dominic Hamon wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, line 111 https://reviews.apache.org/r/25623/diff/2/?file=689080#file689080line111 what happens if i pass in (by accident) a NULL FILE pointer? it might end up here which

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-15 Thread Ian Downes
On Sept. 11, 2014, 4:56 p.m., Vinod Kone wrote: src/slave/containerizer/isolators/filesystem/shared.cpp, line 124 https://reviews.apache.org/r/25549/diff/1/?file=686147#file686147line124 s/host_path/hostPath/ is this style strictly enforced? It matches the snaked cased

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/#review53370 --- Thanks for adding this Kapil! I made a few notes for cleanup, in

Re: Review Request 24264: Installed python libraries during make install.

2014-09-15 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24264/#review53375 --- src/Makefile.am https://reviews.apache.org/r/24264/#comment93020

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Dominic Hamon
On Sept. 15, 2014, 11:46 a.m., Ben Mahler wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp, lines 28-29 https://reviews.apache.org/r/25597/diff/3/?file=688423#file688423line28 Any reason not to take a const ? How about s/str/version/ in this whole

Re: Completed tasks remains in TASK_RUNNING when framework is disconnected

2014-09-15 Thread Benjamin Mahler
To ensure that the architecture of mesos remains a scalable one, we want to persist state in the leaves of the system as much as possible. This is why the master has never persisted tasks, task states, or status updates. Note that status updates can contain arbitrarily large amounts of data at the

Re: Review Request 22980: Enable support for existing cgroups, don't always try to create.

2014-09-15 Thread Timothy St. Clair
On July 15, 2014, 6:03 a.m., Jie Yu wrote: Tim, sorry for getting to your reviews late. I'm really swamped recently. Thank you for your patience. I discovered at least three problems in this patch that needs to be addressed. 1) In CgroupsCpushareIsolatorProcess::cleanup, for

Re: Review Request 24177: Pass executor directory to Isolator::prepare().

2014-09-15 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24177/ --- (Updated Sept. 15, 2014, 12:59 p.m.) Review request for mesos, Ben Mahler and

Review Request 25655: Add alternate os::chown taking uid and gid.

2014-09-15 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25655/ --- Review request for mesos and Vinod Kone. Repository: mesos-git Description

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-15 Thread Ian Downes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/ --- (Updated Sept. 15, 2014, 1:01 p.m.) Review request for mesos, Ben Mahler, Jie

Re: Review Request 25655: Add alternate os::chown taking uid and gid.

2014-09-15 Thread Kamil Domanski
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25655/#review53397 --- 3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp

Re: Review Request 24776: Add docker containerizer destroy tests

2014-09-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24776/ --- (Updated Sept. 15, 2014, 8:33 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 24264: Installed python libraries during make install.

2014-09-15 Thread Thomas Rampelberg
On Sept. 15, 2014, 6:53 p.m., Cody Maloney wrote: src/Makefile.am, line 1043 https://reviews.apache.org/r/24264/diff/11/?file=674261#file674261line1043 Shouldn't we install these globally / not for '--user'? It isn't getting installed for the user. I'm setting the install location

Re: Review Request 25614: Safer handling of futures in JNI state abstraction

2014-09-15 Thread Connor Doyle
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/ --- (Updated Sept. 15, 2014, 1:54 p.m.) Review request for mesos and Niklas

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/#review53403 --- Patch looks great! Reviews applied: [25655, 24177, 25549] All

Re: Review Request 25035: Fix for MESOS-1688

2014-09-15 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25035/#review53362 --- Minor nits and we will get this committed. Thanks for your patience

Re: Review Request 25614: Safer handling of futures in JNI state abstraction

2014-09-15 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/#review53408 --- Ship it! Ship It! - Niklas Nielsen On Sept. 15, 2014, 1:54

Re: Review Request 25614: Safer handling of futures in JNI state abstraction

2014-09-15 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/#review53409 --- This more looks like a bug somewhere. Let's try to find out the

Re: Review Request 25121: Try to give a hint as to what might have gone wrong

2014-09-15 Thread Niklas Nielsen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25121/#review53410 --- Can you add some context to the background of this RR? - Niklas

Re: Review Request 25614: Safer handling of futures in JNI state abstraction

2014-09-15 Thread Niklas Nielsen
On Sept. 15, 2014, 2:17 p.m., Jie Yu wrote: This more looks like a bug somewhere. Let's try to find out the true root cause first. True - but throwing the result away from await() seems silly/unsafe too (rather on relying on the await to have completed or wait forever). Our investigation

Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.

2014-09-15 Thread Jiang Yan Xu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/ --- Review request for mesos and Ben Mahler. Bugs: MESOS-1392

Re: Review Request 25614: Safer handling of futures in JNI state abstraction

2014-09-15 Thread Connor Doyle
On Sept. 15, 2014, 9:17 p.m., Jie Yu wrote: This more looks like a bug somewhere. Let's try to find out the true root cause first. Niklas Nielsen wrote: True - but throwing the result away from await() seems silly/unsafe too (rather on relying on the await to have completed or

Re: Review Request 24776: Add docker containerizer destroy tests

2014-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24776/#review53418 --- Patch looks great! Reviews applied: [24776] All tests passed. -

Re: Review Request 25261: Check for variadic template support

2014-09-15 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25261/ --- (Updated Sept. 15, 2014, 3:03 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 25261: Check for variadic template support

2014-09-15 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25261/ --- (Updated Sept. 15, 2014, 3:03 p.m.) Review request for mesos and Benjamin

Re: Review Request 25549: Basic filesystem isolator for Linux.

2014-09-15 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/#review53404 --- src/slave/slave.cpp

Re: Completed tasks remains in TASK_RUNNING when framework is disconnected

2014-09-15 Thread Niklas Nielsen
Thanks for your input Ben! (Comments inlined) On 15 September 2014 12:35, Benjamin Mahler benjamin.mah...@gmail.com wrote: To ensure that the architecture of mesos remains a scalable one, we want to persist state in the leaves of the system as much as possible. This is why the master has

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

2014-09-15 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Sept. 15, 2014, 10:20 p.m.) Review request for mesos, Benjamin

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/ --- (Updated Sept. 15, 2014, 6:22 p.m.) Review request for mesos, Adam B and

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Kapil Arya
On Sept. 15, 2014, 2:46 p.m., Ben Mahler wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp, lines 32-34 https://reviews.apache.org/r/25597/diff/3/?file=688423#file688423line32 The program will crash if the split is non-numeric, because you'll call .get() on a

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

2014-09-15 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review53420 --- Ship it! Ship It! - Dominic Hamon On Sept. 15, 2014, 3:20 p.m.,

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Kapil Arya
On Sept. 15, 2014, 2:46 p.m., Ben Mahler wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp, lines 86-89 https://reviews.apache.org/r/25597/diff/3/?file=688423#file688423line86 Can you remove all of the _ suffixes for these members? If you look through our

Re: Review Request 25261: Check for variadic template support

2014-09-15 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25261/ --- (Updated Sept. 15, 2014, 3:26 p.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 25663: MESOS-1392: MasterDetector now returns a None when it cannot read the content of the ZNode it has detected.

2014-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25663/#review53429 --- Patch looks great! Reviews applied: [25663] All tests passed. -

Re: Review Request 25614: Safer handling of futures in JNI state abstraction

2014-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25614/#review53440 --- Patch looks great! Reviews applied: [25614] All tests passed. -

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Ben Mahler
On Sept. 15, 2014, 6:46 p.m., Ben Mahler wrote: Thanks for adding this Kapil! I made a few notes for cleanup, in particular, about: (1) The consolidation with os::Release, which implements ~ the same thing. (2) Leveraging stringify with a stream operator as opposed to using a

Re: Review Request 25622: Update the Mesos Style Guide with C++11 and naming notes.

2014-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review53447 --- Patch looks great! Reviews applied: [25622] All tests passed. -

Re: Completed tasks remains in TASK_RUNNING when framework is disconnected

2014-09-15 Thread Benjamin Mahler
On Mon, Sep 15, 2014 at 3:11 PM, Niklas Nielsen nik...@mesosphere.io wrote: Thanks for your input Ben! (Comments inlined) On 15 September 2014 12:35, Benjamin Mahler benjamin.mah...@gmail.com wrote: To ensure that the architecture of mesos remains a scalable one, we want to persist

Re: Review Request 25618: MESOS-1792: add os::shell with pluggable IO descriptors

2014-09-15 Thread Kamil Domanski
On Sept. 15, 2014, 7:34 p.m., Ian Downes wrote: Drive by question: what does this offer that can't already be done with Subprocess? Form is path, vector of args and pipe/path/fd for in/out/err. Huh, I guess I should have familiarized myself a little bit more with the codebase before

Re: Review Request 25614: Safer handling of futures in JNI state abstraction

2014-09-15 Thread Ben Mahler
On Sept. 15, 2014, 9:17 p.m., Jie Yu wrote: This more looks like a bug somewhere. Let's try to find out the true root cause first. Niklas Nielsen wrote: True - but throwing the result away from await() seems silly/unsafe too (rather on relying on the await to have completed or

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/#review53453 --- Bad patch! Reviews applied: [25597] Failed command:

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/ --- (Updated Sept. 15, 2014, 10:02 p.m.) Review request for mesos, Adam B and

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/#review53463 --- 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25597/ --- (Updated Sept. 16, 2014, 1:11 a.m.) Review request for mesos, Adam B and

Re: Review Request 25597: Added a version checker class to stout.

2014-09-15 Thread Kapil Arya
On Sept. 16, 2014, 12:25 a.m., Cody Maloney wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/version.hpp, line 33 https://reviews.apache.org/r/25597/diff/5/?file=690341#file690341line33 Using a C array initializer here would be a little cleaner to read int versions[3] =

Re: Review Request 25270: Enable bridge network in Mesos

2014-09-15 Thread Bhuvan Arumugam
On Sept. 15, 2014, 6 a.m., Derek Zhang wrote: Ship It! any more volunteers to bless submit this patch? - Bhuvan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25270/#review53312