Re: Review Request 26352: Add --with-curl to libprocess since stout/net.hpp needs it.

2014-10-07 Thread Till Toenshoff
On Oct. 6, 2014, 1:30 p.m., Till Toenshoff wrote: 3rdparty/libprocess/configure.ac, line 99 https://reviews.apache.org/r/26352/diff/1/?file=714029#file714029line99 Is there a reason you are not adhering to that without_bundled_xxx scheme we are using for other bundled libraries?

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

2014-10-07 Thread Bernd Mathiske
On Aug. 7, 2014, 5:57 p.m., Vinod Kone wrote: src/tests/slave_tests.cpp, lines 989-998 https://reviews.apache.org/r/23912/diff/4/?file=652899#file652899line989 Do you have to send killTask() before runTask() is called? AFAICT, you just need to make sure to call it before

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

2014-10-07 Thread Bernd Mathiske
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/ --- (Updated Oct. 7, 2014, 8:18 a.m.) Review request for mesos. Changes ---

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-10-07 Thread Alexander Rukletsov
On Oct. 3, 2014, 5:25 p.m., Niklas Nielsen wrote: src/exec/exec.cpp, lines 75-76 https://reviews.apache.org/r/25434/diff/4/?file=709150#file709150line75 Why Java executors? I am not sure I understand this comment. This is what BenH pointed me at. Since Java has garbage collection,

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-10-07 Thread Alexander Rukletsov
On Oct. 2, 2014, 7:49 a.m., Timothy Chen wrote: src/exec/exec.cpp, line 739 https://reviews.apache.org/r/25434/diff/4/?file=709150#file709150line739 I'm not sure this should be at the WARNING level, as it's not really expected to have it set all the time. IMHO it should be INFO

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-10-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25434/ --- (Updated Oct. 7, 2014, 3:54 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

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

Re: Review Request 26229: Expose poll interval from the reaper.

2014-10-07 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26229/ --- (Updated Oct. 7, 2014, 4:39 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 26229: Expose poll interval from the reaper.

2014-10-07 Thread Alexander Rukletsov
On Oct. 6, 2014, 10:04 p.m., Ben Mahler wrote: 3rdparty/libprocess/include/process/reap.hpp, lines 12-14 https://reviews.apache.org/r/26229/diff/1/?file=710087#file710087line12 Can you add some more description to this ticket? If a caller is to use these values, their

Re: Review Request 26229: Expose poll interval from the reaper.

2014-10-07 Thread Alexander Rukletsov
On Oct. 6, 2014, 10:02 p.m., Ben Mahler wrote: 3rdparty/libprocess/src/reap.cpp, lines 124-127 https://reviews.apache.org/r/26229/diff/1/?file=710088#file710088line124 Why do you need a variable for this? Can't this just be a 'return' statement? If there's a reason

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

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

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

2014-10-07 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55665 --- I don't see any changes in the diff? Bad diff? - Vinod Kone On

Re: Review Request 25945: Pass Promise to DispatchEvent to correctly fail on rejection.

2014-10-07 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25945/ --- (Updated Oct. 7, 2014, 10:51 a.m.) Review request for mesos, Benjamin Hindman

Re: Review Request 25434: Propagate slave shutdown grace period to Executor and CommandExecutor.

2014-10-07 Thread Timothy Chen
On Oct. 2, 2014, 7:49 a.m., Timothy Chen wrote: src/tests/slave_tests.cpp, line 1023 https://reviews.apache.org/r/25434/diff/4/?file=709164#file709164line1023 Times(1) is default, can ignore the call. Alexander Rukletsov wrote: To the best of my knowledge, we always

Review Request 26420: Use RBT by default and correctly determine if version is = 0.6

2014-10-07 Thread Dominic Hamon
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26420/ --- Review request for mesos and Vinod Kone. Repository: mesos-git Description

Re: Review Request 26273: Define reverseforeach from Boost.

2014-10-07 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26273/#review55672 --- I am also against this because not every std container has

Re: Review Request 23912: Fix MESOS-947: Slave should properly handle a killTask() that arrives between runTask() and _runTask()

2014-10-07 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55671 --- src/slave/slave.cpp

Re: Review Request 26229: Expose poll interval from the reaper.

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

Review Request 26421: Add references to native packages available in downstream channels.

2014-10-07 Thread Timothy St. Clair
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26421/ --- Review request for mesos and Dave Lester. Repository: mesos-git Description

Re: Review Request 26352: Add --with-curl to libprocess since stout/net.hpp needs it.

2014-10-07 Thread Timothy St. Clair
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26352/#review55678 --- Ship it! Ship It! - Timothy St. Clair On Oct. 6, 2014, 1:50

Re: Review Request 26229: Expose poll interval from the reaper.

2014-10-07 Thread Michael Park
On Oct. 6, 2014, 10:02 p.m., Ben Mahler wrote: 3rdparty/libprocess/src/reap.cpp, lines 124-127 https://reviews.apache.org/r/26229/diff/1/?file=710088#file710088line124 Why do you need a variable for this? Can't this just be a 'return' statement? If there's a reason

Re: Review Request 26396: Exposed p90/95/99 for tcp rtt container stats.

2014-10-07 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26396/#review55677 --- src/slave/containerizer/isolators/network/port_mapping.cpp

Re: Review Request 25861: Serialize isolator prepare and cleanup (reversed).

2014-10-07 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25861/#review55674 --- src/slave/containerizer/mesos/containerizer.cpp

Re: Review Request 24469: Created pure python package for the CLI.

2014-10-07 Thread Thomas Rampelberg
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24469/ --- (Updated Oct. 7, 2014, 6:57 p.m.) Review request for mesos and Benjamin

Re: Review Request 24469: Created pure python package for the CLI.

2014-10-07 Thread Thomas Rampelberg
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24469/ --- (Updated Oct. 7, 2014, 6:57 p.m.) Review request for mesos and Benjamin

Review Request 26423: Added documentation for egress rate limit control

2014-10-07 Thread Chi Zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26423/ --- Review request for mesos, Ian Downes, Jie Yu, and Cong Wang. Repository:

Re: Review Request 25551: Add standard versioning to shared libmesos.so

2014-10-07 Thread Timothy St. Clair
On Sept. 26, 2014, 5:22 p.m., Vinod Kone wrote: src/Makefile.am, line 567 https://reviews.apache.org/r/25551/diff/2/?file=706149#file706149line567 why not just libmesos_la_LDFLAGS = -version-info $(PACKAGE-VERSION) More importantly how do the resulting

Re: Review Request 25551: Add standard versioning to shared libmesos.so

2014-10-07 Thread Timothy St. Clair
On Sept. 27, 2014, 12:09 a.m., Kapil Arya wrote: src/Makefile.am, line 567 https://reviews.apache.org/r/25551/diff/2/?file=706149#file706149line567 Dumb question: Shouldn't MAJOR/MINOR be swapped? I'm pretty sure it's correct, but the api is... obtuse. - Timothy

Supporting Stateful Services on Mesos (MESOS-1554)

2014-10-07 Thread Jie Yu
Hi, We're actively seeking to build a few primitives in Mesos which would allow us to run stateful services (like HDFS) on Mesos (see MESOS-1554 https://issues.apache.org/jira/browse/MESOS-1554). We've been iterating on the design for a while (mainly Mesosphere and Twitter). We've put together

Re: Review Request 25865: Pid namespace isolator for the MesosContainerizer.

2014-10-07 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25865/#review55685 --- Vinod and Jie asked me to take a look at this review. It looks

Re: Review Request 26421: Add references to native packages available in downstream channels.

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

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

2014-10-07 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25549/#review55689 --- LGTM overall. I'll give a shipit once the following comments are

Re: Review Request 26229: Expose poll interval from the reaper.

2014-10-07 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26229/#review55718 --- I'm curious why you need to expose both sides of the bounds? Our

Review Request 26426: Add --enable-debug flag to ./configure for controlling emission of debug information

2014-10-07 Thread Cody Maloney
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26426/ --- Review request for mesos, Benjamin Hindman and Timothy St. Clair. Repository:

Re: Review Request 25848: Introducing mesos modules.

2014-10-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Oct. 7, 2014, 6:43 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 25848: Introducing mesos modules.

2014-10-07 Thread Kapil Arya
On Oct. 6, 2014, 11:56 p.m., Benjamin Hindman wrote: src/examples/example_module_impl.cpp, line 32 https://reviews.apache.org/r/25848/diff/14/?file=713786#file713786line32 To Bernd's point above, if you declare/define this at the bottom then you won't need the forward

Re: Review Request 25848: Introducing mesos modules.

2014-10-07 Thread Kapil Arya
On Oct. 6, 2014, 7:25 a.m., Bernd Mathiske wrote: src/examples/test_module.hpp, line 26 https://reviews.apache.org/r/25848/diff/14/?file=713787#file713787line26 Now there is a confusion whether TestModule is the module or exampleModule is the module. I suggest we only call one of

Re: Review Request 25848: Introducing mesos modules.

2014-10-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Oct. 7, 2014, 6:59 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 25848: Introducing mesos modules.

2014-10-07 Thread Kapil Arya
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/ --- (Updated Oct. 7, 2014, 7:07 p.m.) Review request for mesos, Benjamin Hindman,

Re: Review Request 24469: Created pure python package for the CLI.

2014-10-07 Thread Thomas Rampelberg
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24469/ --- (Updated Oct. 7, 2014, 11:09 p.m.) Review request for mesos and Benjamin

Re: Review Request 26423: Added documentation for egress rate limit control

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

Re: Review Request 26426: Add --enable-debug flag to ./configure for controlling emission of debug information

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

Re: Review Request 26289: Replace some shared_ptr with Owned to clarify ownership passing.

2014-10-07 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26289/#review55748 --- Can you use unique_ptr directly now? If yes, I would suggest using

Re: Review Request 25848: Introducing mesos modules.

2014-10-07 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848/#review55735 --- src/module/manager.hpp

Re: Review Request 26289: Replace some shared_ptr with Owned to clarify ownership passing.

2014-10-07 Thread Michael Park
On Oct. 8, 2014, 12:32 a.m., Jie Yu wrote: Can you use unique_ptr directly now? If yes, I would suggest using unique_ptr directly in libprocess. Here are the reasons: 1) We use lots of shared_ptr in libprocess. Mixing Owned with shared_ptr seems to be confusing. 2) What if the

Re: Review Request 26242: Changed checkpoint logging from LOG(INFO) to VLOG(1).

2014-10-07 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26242/#review55760 --- Ship it! Ship It! - Ben Mahler On Oct. 1, 2014, 7:07 p.m.,

Build failed in Jenkins: mesos-reviewbot #1875

2014-10-07 Thread Apache Jenkins Server
See https://builds.apache.org/job/mesos-reviewbot/1875/ -- [...truncated 5254 lines...] rm -f tests/common/.deps/.dirstamp rm -f tests/common/.dirstamp rm -f usage/.deps/.dirstamp rm -f usage/.dirstamp rm -f zookeeper/.deps/.dirstamp rm -f

Re: Review Request 25848: Introducing mesos modules.

2014-10-07 Thread Benjamin Hindman
On Oct. 8, 2014, 1:17 a.m., Michael Park wrote: src/module/manager.hpp, lines 95-105 https://reviews.apache.org/r/25848/diff/17/?file=714903#file714903line95 I would suggest changing these to be static functions that return static singletons as per

Re: Review Request 24469: Created pure python package for the CLI.

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

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

2014-10-07 Thread Apache Jenkins Server
See https://builds.apache.org/job/mesos-reviewbot/1876/

Review Request 26436: Avoid docker inspect on each usage call

2014-10-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26436/ --- Review request for mesos and Benjamin Hindman. Repository: mesos-git

Re: Review Request 26436: Avoid docker inspect on each usage call

2014-10-07 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26436/ --- (Updated Oct. 8, 2014, 4:51 a.m.) Review request for mesos and Benjamin

Re: Review Request 26436: Avoid docker inspect on each usage call

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