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

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

Review Request 26486: Fix containerizer not receiving destroy/update calls when launching

2014-10-09 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26486/ --- Review request for mesos and Benjamin Hindman. Bugs: MESOS-1884

Re: Review Request 26486: Fix containerizer not receiving destroy/update calls when launching

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

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

2014-10-09 Thread Michael Park
--- 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 25184: Delete framework data in TaskStatus to avoid OOM

2014-10-09 Thread Chengwei Yang
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 19180: Fix mesos command parsing help

2014-10-09 Thread Chengwei Yang
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 available

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

2014-10-09 Thread Mesos ReviewBot
--- 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. -

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

2014-10-09 Thread Adam B
On Oct. 8, 2014, 8:49 p.m., Dominic Hamon wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 https://reviews.apache.org/r/26472/diff/1/?file=716305#file716305line36 it might be worth considering using stringstream here to build the same error message

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

2014-10-09 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26473/#review55963 --- Ship it! Minor nit, question

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

2014-10-09 Thread Adam B
--- 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

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

2014-10-09 Thread Alexander Rukletsov
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 relaunch of the

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

2014-10-09 Thread Bernd Mathiske
On Oct. 8, 2014, 10:42 a.m., Vinod Kone wrote: src/tests/slave_tests.cpp, lines 1071-1075 https://reviews.apache.org/r/23912/diff/6-7/?file=714593#file714593line1071 This has not been moved !? I have been searching through the previous review comments, but have not been able to

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

2014-10-09 Thread Dominic Hamon
--- 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 26382: [WIP] Annotate TASK_LOST with source. Consider TASK_INVALID for some cases.

2014-10-09 Thread Dominic Hamon
--- 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.

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

2014-10-09 Thread Bernd Mathiske
On Aug. 5, 2014, 2:29 p.m., Vinod Kone wrote: src/tests/slave_tests.cpp, lines 958-962 https://reviews.apache.org/r/23912/diff/3/?file=643815#file643815line958 Kill these expectations since an executor won't be launched in this test. Vinod Kone wrote: doesnt look like

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

2014-10-09 Thread Bernd Mathiske
--- 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()

2014-10-09 Thread Mesos ReviewBot
--- 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. -

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

2014-10-09 Thread minid33
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

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

2014-10-09 Thread Timothy Chen
--- 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

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

2014-10-09 Thread Timothy Chen
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:

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

2014-10-09 Thread Alexander Rukletsov
--- 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,

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

2014-10-09 Thread Cody Maloney
--- 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 26473: libprocess: Always use stout ABORT() rather than system abort()

2014-10-09 Thread Cody Maloney
--- 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()

2014-10-09 Thread Dominic Hamon
On Oct. 8, 2014, 8:49 p.m., Dominic Hamon wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 https://reviews.apache.org/r/26472/diff/1/?file=716305#file716305line36 it might be worth considering using stringstream here to build the same error message

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

2014-10-09 Thread Mesos ReviewBot
--- 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. -

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

2014-10-09 Thread Cody Maloney
On Oct. 9, 2014, 3:49 a.m., Dominic Hamon wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 https://reviews.apache.org/r/26472/diff/1/?file=716305#file716305line36 it might be worth considering using stringstream here to build the same error message

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

2014-10-09 Thread Dominic Hamon
On Oct. 8, 2014, 8:49 p.m., Dominic Hamon wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/stringify.hpp, line 36 https://reviews.apache.org/r/26472/diff/1/?file=716305#file716305line36 it might be worth considering using stringstream here to build the same error message

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

2014-10-09 Thread Kapil Arya
--- 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.

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

2014-10-09 Thread Kapil Arya
--- 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

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

2014-10-09 Thread Cody Maloney
On Oct. 9, 2014, 2:05 p.m., Dominic Hamon wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp, line 131 https://reviews.apache.org/r/26476/diff/1/?file=716336#file716336line131 std::unique_ptr would also be an option as it can be moved on Option copy. Would that be

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

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

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

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

Review Request 26513: Disallow duplicate module names.

2014-10-09 Thread Kapil Arya
--- 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

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

2014-10-09 Thread Kapil Arya
--- 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,

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

2014-10-09 Thread Kapil Arya
--- 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 26229: Expose poll interval from the reaper.

2014-10-09 Thread Mesos ReviewBot
--- 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. -

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

2014-10-09 Thread Timothy Chen
--- 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 26513: Disallow duplicate module names.

2014-10-09 Thread Kapil Arya
--- 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 26513: Disallow duplicate module names.

2014-10-09 Thread Niklas Nielsen
--- 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 -

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

2014-10-09 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 26473: libprocess: Always use stout ABORT() rather than system abort()

2014-10-09 Thread Mesos ReviewBot
--- 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

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

2014-10-09 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26487/#review56057 --- Ah, I suspected this would happen.. =/

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

2014-10-09 Thread Timothy Chen
--- 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

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

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

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

2014-10-09 Thread Ben Mahler
--- 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.

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

2014-10-09 Thread Ben Mahler
--- 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

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

2014-10-09 Thread Mesos ReviewBot
--- 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

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

2014-10-09 Thread Ian Downes
On Oct. 7, 2014, 2:44 p.m., Jie Yu wrote: src/slave/containerizer/isolators/filesystem/shared.cpp, lines 74-77 https://reviews.apache.org/r/25549/diff/5/?file=711209#file711209line74 Should we check other error conditions as well? For example,

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

2014-10-09 Thread Ian Downes
On Oct. 2, 2014, 12:12 p.m., Timothy St. Clair wrote: src/slave/containerizer/isolators/filesystem/shared.cpp, line 90 https://reviews.apache.org/r/25865/diff/3/?file=711226#file711226line90 Why not use api's and MS_REMOUNT? This is run in the forked child process. This code has

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

2014-10-09 Thread Ian Downes
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 really

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

2014-10-09 Thread Kapil Arya
--- 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,

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

2014-10-09 Thread Mesos ReviewBot
--- 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

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

2014-10-09 Thread Cody Maloney
On Oct. 9, 2014, 9:59 p.m., Ben Mahler wrote: 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp, lines 442-443 https://reviews.apache.org/r/26472/diff/2/?file=716714#file716714line442 Hm.. could we avoid the assumption that stringify() keeps errno intact? ```

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

2014-10-09 Thread Cody Maloney
--- 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

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

2014-10-09 Thread Cody Maloney
--- 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

Review Request 26533: Memory cleanup: libprocess finalize

2014-10-09 Thread Joris Van Remoortere
--- 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:

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

2014-10-09 Thread Timothy Chen
--- 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

Re: Review Request 26533: Memory cleanup: libprocess finalize

2014-10-09 Thread Mesos ReviewBot
--- 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. -

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

2014-10-09 Thread Ben Mahler
--- 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 25865: Pid namespace isolator for the MesosContainerizer.

2014-10-09 Thread Ben Mahler
On Oct. 7, 2014, 9:23 p.m., Ben Mahler wrote: src/slave/containerizer/linux_launcher.cpp, line 362 https://reviews.apache.org/r/25865/diff/3/?file=711229#file711229line362 This line is unused...? Ian Downes wrote: It's used in the os::getns call below. I capture it so I can

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

2014-10-09 Thread Vinod Kone
--- 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 25525: MESOS-1739: Allow slave reconfiguration on restart

2014-10-09 Thread Cody Maloney
--- 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

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

2014-10-09 Thread Cody Maloney
--- 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

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

2014-10-09 Thread Mesos ReviewBot
--- 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. -

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

2014-10-09 Thread Mesos ReviewBot
--- 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. -