Re: Review Request 26517: Symlink sandbox directories in docker containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26517/ --- (Updated Oct. 10, 2014, 6:35 a.m.) Review request for mesos and Benjamin Hindma

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/#review56104 --- Patch looks great! Reviews applied: [25525] All tests passed. - M

Re: Review Request 26535: Added batch sizes and timings to the Registrar logging.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/#review56097 --- Patch looks great! Reviews applied: [26535] All tests passed. - M

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 10, 2014, 2:29 a.m.) Review request for mesos, Adam B and Vinod K

Re: Review Request 25525: MESOS-1739: Allow slave reconfiguration on restart

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25525/ --- (Updated Oct. 10, 2014, 2:29 a.m.) Review request for mesos, Adam B, Benjamin H

Re: Review Request 26535: Added batch sizes and timings to the Registrar logging.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/#review56093 --- Ship it! Ship It! - Vinod Kone On Oct. 10, 2014, 1:51 a.m., Ben

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

> On Oct. 7, 2014, 9:23 p.m., Ben Mahler wrote: > > src/slave/containerizer/linux_launcher.cpp, line 362 > > > > > > This line is unused...? > > Ian Downes wrote: > It's used in the os::getns call below. I capture

Review Request 26535: Added batch sizes and timings to the Registrar logging.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26535/ --- Review request for mesos and Vinod Kone. Repository: mesos-git Description --

Re: Review Request 26533: Memory cleanup: libprocess finalize

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/#review56091 --- Patch looks great! Reviews applied: [26533] All tests passed. - M

Re: Review Request 26517: Symlink sandbox directories in docker containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26517/ --- (Updated Oct. 10, 2014, 1:22 a.m.) Review request for mesos and Benjamin Hindma

Review Request 26533: Memory cleanup: libprocess finalize

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26533/ --- Review request for mesos, Benjamin Hindman and Niklas Nielsen. Repository: meso

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26472/ --- (Updated Oct. 10, 2014, 12:46 a.m.) Review request for mesos, Adam B and Domini

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26472/ --- (Updated Oct. 10, 2014, 12:44 a.m.) Review request for mesos, Adam B and Domini

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

> On Oct. 9, 2014, 9:59 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, lines 442-443 > > > > > > Hm.. could we avoid the assumption that stringify() keeps errno intact? > > >

Re: Review Request 26509: Added --module flag for Mesos slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56085 --- Bad patch! Reviews applied: [26529, 26513] Failed command: git app

Re: Review Request 26509: Added --module flag for Mesos slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 9, 2014, 7:19 p.m.) Review request for mesos and Niklas Nielsen.

Re: Review Request 26508: Added --module flag for Mesos master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 7:19 p.m.) Review request for mesos, Benjamin Hindman, B

Re: Review Request 26529: Modules should accept relative path or filename for library name.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- (Updated Oct. 9, 2014, 7:19 p.m.) Review request for mesos and Niklas Nielsen.

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

> On Oct. 7, 2014, 2:23 p.m., Ben Mahler wrote: > > Vinod and Jie asked me to take a look at this review. > > > > It looks like there are dependent changes that are not linked in? > > (filesystem islotor, ContainerInfo::MESOS, os::namespaces os::getns). > > > > My main comment is that it seems

Re: Review Request 26509: Added --module flag for Mesos slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56075 --- Bad patch! Reviews applied: [26529, 26513] Failed command: git app

Re: Review Request 26508: Added --module flag for Mesos master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 6:29 p.m.) Review request for mesos, Benjamin Hindman, B

Re: Review Request 26509: Added --module flag for Mesos slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 9, 2014, 6:29 p.m.) Review request for mesos and Niklas Nielsen.

Review Request 26529: Modules should accept relative path or filename for library name.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26529/ --- Review request for mesos and Niklas Nielsen. Repository: mesos-git Descriptio

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

> On Oct. 7, 2014, 9:44 p.m., Jie Yu wrote: > > src/slave/containerizer/mesos/containerizer.cpp, lines 434-440 > > > > > > This check is redundent, right? > > Ian Downes wrote: > Redundant with what? This is retur

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

> On Oct. 2, 2014, 12:12 p.m., Timothy St. Clair wrote: > > src/slave/containerizer/isolators/filesystem/shared.cpp, line 90 > > > > > > Why not use api's and MS_REMOUNT? This is run in the forked child process. This c

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

> On Oct. 7, 2014, 2:44 p.m., Jie Yu wrote: > > src/slave/containerizer/isolators/filesystem/shared.cpp, lines 74-77 > > > > > > Should we check other error conditions as well? > > > > For example, `!executorIn

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26472/#review56060 --- Looks good, just two minor issues below. Deferring to adam and domi

Re: Review Request 26525: Moved executor checkpointing code from the Executor constructor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26525/#review56061 --- Bad patch! Reviews applied: [26525] Failed command: git apply --in

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/#review56058 --- Ship it! Ok, looks good to me, modulo the comment. docs/mesos-c++

Review Request 26525: Moved executor checkpointing code from the Executor constructor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26525/ --- Review request for mesos, Ben Mahler and Vinod Kone. Repository: mesos-git De

Re: Review Request 26517: Symlink sandbox directories in docker containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26517/#review56059 --- Bad patch! Reviews applied: [26517] Failed command: ./support/meso

Re: Review Request 26487: Perform read right after subprocess for docker ps

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26487/ --- (Updated Oct. 9, 2014, 9:38 p.m.) Review request for mesos and Benjamin Hindman

Re: Review Request 26487: Perform read right after subprocess for docker ps

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26487/#review56057 --- Ah, I suspected this would happen.. =/ https://issues.apache.org/jir

Re: Review Request 26473: libprocess: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26473/#review56053 --- Patch looks great! Reviews applied: [26472, 26473] All tests passe

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

> On Sept. 26, 2014, 5:22 p.m., Vinod Kone wrote: > > src/Makefile.am, line 567 > > > > > > why not just > > > > libmesos_la_LDFLAGS = -version-info $(PACKAGE-VERSION) > > > > More importantly how do

Review Request 26517: Symlink sandbox directories in docker containerizer

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26517/ --- Review request for mesos and Benjamin Hindman. Repository: mesos-git Descript

Re: Review Request 26509: Added --module flag for Mesos slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56044 --- Bad patch! Reviews applied: [26513, 26513] Failed command: git app

Re: Review Request 26513: Disallow duplicate module names.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26513/#review56043 --- Ship it! Looks good! Will get this committed for you shortly - Nik

Re: Review Request 25250: Mark running tasks killed during framework shutdown.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56042 --- src/tests/master_tests.cpp

Re: Review Request 26513: Disallow duplicate module names.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26513/ --- (Updated Oct. 9, 2014, 3:59 p.m.) Review request for mesos and Niklas Nielsen.

Re: Review Request 25250: Mark running tasks killed during framework shutdown.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/#review56041 --- src/tests/master_tests.cpp

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26229/#review56038 --- Patch looks great! Reviews applied: [26229] All tests passed. - M

Re: Review Request 26473: libprocess: Always use stout ABORT() rather than system abort()

> On Oct. 9, 2014, 9:14 a.m., Adam B wrote: > > 3rdparty/libprocess/src/httpd.cpp, lines 32-37 > > > > > > Why are you removing this here? Doesn't seem related to the abort > > change. Or did you just notice that these

Re: Review Request 26513: Disallow duplicate module names.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26513/#review56034 --- Looks good! Only minor nits src/module/manager.cpp

Re: Review Request 26508: Added --module flag for Mesos master.

> On Oct. 9, 2014, 2:15 p.m., Niklas Nielsen wrote: > > src/module/manager.hpp, lines 30-31 > > > > > > Is this just a fly-by style fix? > > Why include module.hpp here? module/manager.hpp uses mesos::ModuleBase ty

Re: Review Request 26509: Added --module flag for Mesos slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- (Updated Oct. 9, 2014, 3:07 p.m.) Review request for mesos and Niklas Nielsen.

Re: Review Request 26508: Added --module flag for Mesos master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- (Updated Oct. 9, 2014, 3:05 p.m.) Review request for mesos, Benjamin Hindman, B

Review Request 26513: Disallow duplicate module names.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26513/ --- Review request for mesos and Niklas Nielsen. Repository: mesos-git Descriptio

Re: Review Request 26509: Added --module flag for Mesos slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/#review56024 --- src/slave/flags.hpp

Re: Review Request 26508: Added --module flag for Mesos master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/#review56013 --- src/master/flags.hpp

Re: Review Request 26476: Remove dynamic allocation from Option.

> On Oct. 9, 2014, 2:05 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp, line 131 > > > > > > std::unique_ptr would also be an option as it can be moved on Option > > copy. W

Review Request 26509: Added --module flag for Mesos slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26509/ --- Review request for mesos and Niklas Nielsen. Repository: mesos-git Descriptio

Review Request 26508: Added --module flag for Mesos master.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26508/ --- Review request for mesos, Benjamin Hindman, Bernd Mathiske, and Niklas Nielsen.

Re: Review Request 25250: Mark running tasks killed during framework shutdown.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 9, 2014, 5:05 p.m.) Review request for mesos, Benjamin Hindman, B

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26472/#review56007 --- Ship it! Ship It! - Dominic Hamon On Oct. 9, 2014, 9:35 a.m., Co

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

> On Oct. 9, 2014, 3:49 a.m., Dominic Hamon wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 > > > > > > it might be worth considering using stringstream here to build the same > > err

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

> On Oct. 8, 2014, 8:49 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 > > > > > > it might be worth considering using stringstream here to build the same > > err

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

> On Oct. 9, 2014, 3:49 a.m., Dominic Hamon wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 > > > > > > it might be worth considering using stringstream here to build the same > > err

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26436/#review56000 --- Patch looks great! Reviews applied: [26436] All tests passed. - M

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

> On Oct. 8, 2014, 8:49 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 > > > > > > it might be worth considering using stringstream here to build the same > > err

Re: Review Request 26473: libprocess: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26473/ --- (Updated Oct. 9, 2014, 4:38 p.m.) Review request for mesos, Adam B and Dominic

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26472/ --- (Updated Oct. 9, 2014, 4:35 p.m.) Review request for mesos, Adam B and Dominic

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26229/ --- (Updated Oct. 9, 2014, 4:34 p.m.) Review request for mesos, Benjamin Hindman, B

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25622/ --- (Updated Oct. 9, 2014, 4:31 p.m.) Review request for mesos, Benjamin Hindman, B

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

> On Oct. 9, 2014, 3:49 a.m., Dominic Hamon wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 > > > > > > it might be worth considering using stringstream here to build the same > > err

Re: Review Request 25250: Mark running tasks killed during framework shutdown.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25250/ --- (Updated Oct. 9, 2014, 4:28 p.m.) Review request for mesos, Benjamin Hindman an

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

> On Oct. 9, 2014, 7:07 a.m., Michael Park wrote: > > LGTM, leaving `Ship It`s for committers. Thanks for reviewing! You did catch very good spots. - Timothy --- This is an automatically generated e-mail. To reply, visit: https://review

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26436/ --- (Updated Oct. 9, 2014, 3:56 p.m.) Review request for mesos and Benjamin Hindman

[GitHub] mesos pull request: Added option to enable privileged mode for Doc...

Github user minid33 commented on the pull request: https://github.com/apache/mesos/pull/26#issuecomment-58530671 @kmatzen Looks like this is the way your code enters an Apache project, I'd love for this change to go in, I need it a lot. --- If your project is set up for it, you can r

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23912/#review55993 --- Patch looks great! Reviews applied: [23912] All tests passed. - M

Re: Review Request 25184: Delete framework data in TaskStatus to avoid OOM

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25184/#review55983 --- Patch looks great! Reviews applied: [25184] All tests passed. - M

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

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

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

> On Aug. 5, 2014, 2:29 p.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp, lines 958-962 > > > > > > Kill these expectations since an executor won't be launched in this > > test. > > Vinod Kone wrote: > doesn

Re: Review Request 26382: [WIP] Annotate TASK_LOST with source. Consider TASK_INVALID for some cases.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26382/ --- (Updated Oct. 9, 2014, 7:07 a.m.) Review request for mesos and Vinod Kone. Ch

Re: Review Request 26476: Remove dynamic allocation from Option.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26476/#review55977 --- 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp

Re: Review Request 25184: Delete framework data in TaskStatus to avoid OOM

> On Sept. 27, 2014, 12:47 a.m., Timothy Chen wrote: > > src/master/master.cpp, line 3181 > > > > > > Period in the end of the comment. I'm not sure if I understand you correctly, if not please correct me. Did you m

Re: Review Request 25184: Delete framework data in TaskStatus to avoid OOM

> On Sept. 27, 2014, 1:17 a.m., Adam B wrote: > > src/master/master.cpp, line 3173 > > > > > > Update Brenden's TODO, since you're now wiping the data field. > > I'm not sure if we really want to wipe the message

Re: Review Request 25184: Delete framework data in TaskStatus to avoid OOM

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25184/ --- (Updated Oct. 9, 2014, 10 p.m.) Review request for mesos, Adam B and Timothy St

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

> On Oct. 8, 2014, 10:42 a.m., Vinod Kone wrote: > > src/tests/slave_tests.cpp, lines 1071-1075 > > > > > > This has not been moved !? I have been searching through the previous review comments, but have not been

Re: Review Request 26382: [WIP] Annotate TASK_LOST with source. Consider TASK_INVALID for some cases.

> On Oct. 7, 2014, 6:50 p.m., Vinod Kone wrote: > > OK. Here is a proposal for what it could look like. > > > > > > General idea: We should add as few top level task states as possible > > because it is more work for frameworks. TASK_LOST should be used for cases > > where we expect a relaunc

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26472/#review55961 --- Ship it! Please fix the shadowed variable and maybe the minor align

Re: Review Request 26473: libprocess: Always use stout ABORT() rather than system abort()

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26473/#review55963 --- Ship it! Minor nit, question 3rdparty/libprocess/include/process/

Re: Review Request 26472: stout: Always use stout ABORT() rather than system abort()

> On Oct. 8, 2014, 8:49 p.m., Dominic Hamon wrote: > > 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 > > > > > > it might be worth considering using stringstream here to build the same > > err

Re: Review Request 26487: Perform read right after subprocess for docker ps

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26487/#review55960 --- Patch looks great! Reviews applied: [26487] All tests passed. - M

Re: Review Request 19180: Fix mesos command parsing help

> On May 16, 2014, 7:32 a.m., Niklas Nielsen wrote: > > Hey Chengwei - sorry for the tardy turnaround time on this review request. > > > > To me, it still seems like we are treating the symptoms of the real issue: > > PATH is appended multiple times and the subsequent globbing adds the > > ava

Re: Review Request 25184: Delete framework data in TaskStatus to avoid OOM

> On Oct. 7, 2014, 12:14 a.m., Timothy Chen wrote: > > Chengwei are you still able to work on this patch ? Will like to see this > > get merged in 0.21 @Timothy, sorry to late, I have a one week holiday last week, will update this patch within this week. - Chengwei

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26436/#review55955 --- src/slave/containerizer/docker.cpp

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26436/#review55956 --- LGTM, leaving `Ship It`s for committers. - Michael Park On Oct. 8