Re: Review Request 33109: Allow setting environment variables in mesos-execute

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

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

> On May 12, 2015, 1:58 a.m., Bernd Mathiske wrote: > > src/docker/docker.cpp, line 305 > > > > > > 1. There is no declaration in the hpp file with a comment and there is > > no explanation here. I would suggest us

Re: Review Request 31838: Fixed authentication failure triggered slave crash.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31838/#review83377 --- is this review still active? - Joerg Schad On March 9, 2015, 12:5

Re: Review Request 33513: Added a modules-aware factory for allocators.

> On May 11, 2015, 11:26 a.m., Niklas Nielsen wrote: > > src/master/allocator/allocator.cpp, line 39 > > > > > > Unused variable? Did you want to return this somewhere? Ping - let's get this committed :) - Niklas -

Re: Review Request 29947: Fixed a race condition in hook tests for remove-executor hook.

> On May 8, 2015, 8:47 a.m., Niklas Nielsen wrote: > > Kapil, will you be updating this review to use the libprocess event capture > > instead of the shared promise? Kapil: ping - Niklas --- This is an automatically generated e-mail. T

Re: Review Request 30339: Use flags.hooks.isSome() before calling hooks.

> On March 4, 2015, 4:03 p.m., Niklas Nielsen wrote: > > Do you want this in? If so, please update the review :) - Niklas --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30339/#review72824 --

Re: Review Request 32834: Modifiy gdb scripts error message to check gdb is installed.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32834/#review83422 --- Ship it! Modulo my comment below and Adam's comment. bin/gdb-meso

Re: Review Request 32587: Stopped using RunTaskMessage.framework_id.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32587/#review83423 --- Hey Kapil; what's blocking this patch? - Niklas Nielsen On April

Re: Review Request 33208: Delete detector in MesosSchedulerDriver::stop

> On April 19, 2015, 10:55 p.m., Adam B wrote: > > LGTM, barring a question about ordering/synchronization. I'll let another > > committer take a look before we commit it. > > Adam B wrote: > Would also like to see a successful ReviewBot pass. That MasterFailover > segfault seems like it c

Re: Review Request 32587: Stopped using RunTaskMessage.framework_id.

> On May 12, 2015, 9:45 a.m., Niklas Nielsen wrote: > > Hey Kapil; what's blocking this patch? As part of the deprecation cycle, we need to wait until 0.24 for this patch. All the relevant 0.23 patches have landed. "NOTE: This patch is only to be merged _ONLY_ after all the dependent patches h

Re: Review Request 30609: Added a function that reports file size, not following links.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30609/#review83428 --- 3rdparty/libprocess/3rdparty/stout/include/stout/os/stat.hpp

Re: Review Request 29748: Added tests for dynamic reservation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29748/ --- (Updated May 12, 2015, 5:13 p.m.) Review request for mesos, Alexander Rukletsov

Re: Review Request 29748: Added tests for dynamic reservation.

> On May 11, 2015, 11:32 p.m., Jie Yu wrote: > > src/tests/reservation_tests.cpp, line 365 > > > > > > YOu do need a snake_case checker:) Sigh... Sorry :( Fixed. > On May 11, 2015, 11:32 p.m., Jie Yu wrote: > > src

Re: Review Request 29748: Added tests for dynamic reservation.

> On May 11, 2015, 11:32 p.m., Jie Yu wrote: > > src/tests/reservation_tests.cpp, lines 399-400 > > > > > > Hum, have the following 3 tests already been captured in > > ReserveOperationValidationTest? > > Michael Pa

Re: Review Request 32587: Stopped using RunTaskMessage.framework_id.

> On May 12, 2015, 12:45 p.m., Niklas Nielsen wrote: > > Hey Kapil; what's blocking this patch? > > Adam B wrote: > As part of the deprecation cycle, we need to wait until 0.24 for this > patch. All the relevant 0.23 patches have landed. > "NOTE: This patch is only to be merged _ONLY_ a

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

> On May 11, 2015, 10:19 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 155 > > > > > > unique_ptr is not a POD, so this will still try to run the destructor > > of the function during exit of the program. > >

Re: Review Request 33746: Improve logging of clone flags for linux_launcher.

> On May 1, 2015, 10:43 a.m., Jie Yu wrote: > > src/linux/ns.hpp, line 305 > > > > > > Consider s/stringify/names/ ? I'd expect names() to return a vector which isn't what this does. Changed to stringify(int flags).

Re: Review Request 29748: Added tests for dynamic reservation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29748/ --- (Updated May 12, 2015, 6:41 p.m.) Review request for mesos, Alexander Rukletsov

Re: Review Request 29748: Added tests for dynamic reservation.

> On May 11, 2015, 11:32 p.m., Jie Yu wrote: > > src/tests/reservation_tests.cpp, lines 399-400 > > > > > > Hum, have the following 3 tests already been captured in > > ReserveOperationValidationTest? > > Michael Pa

Re: Review Request 32398: Persisted the reservation state on the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32398/ --- (Updated May 12, 2015, 6:44 p.m.) Review request for mesos, Alexander Rukletsov

Re: Review Request 33152: Moved the slave shutdown test into slave_tests.cpp.

> On April 17, 2015, 12:01 p.m., Alexander Rojas wrote: > > src/tests/slave_tests.cpp, lines 146-148 > > > > > > One line break too many. forgot to publish the updated diff? - Vinod ---

Re: Review Request 33271: Update style guide to disallow capturing temporaries by reference.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33271/#review83461 --- Ship it! Left a minor suggestion but otherwise it looks good to me!

Re: Review Request 32891: Support for entering and configuring a Linux chroot.

> On April 13, 2015, 2:30 p.m., Jie Yu wrote: > > src/linux/fs.cpp, line 432 > > > > > > Copy comments from the other review, can you support `Option > > options` for fs::mount as well? Decided to add as an alternate

Re: Review Request 33154: Added reason metrics for slave removals.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33154/#review83463 --- LGTM. Can you add a comment on removeSlave() on why it takes a const

Re: Review Request 32398: Persisted the reservation state on the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32398/#review83464 --- Patch looks great! Reviews applied: [32139, 32140, 32149, 32150, 29

Re: Review Request 33241: docs: Add documentation on observability metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33241/ --- (Updated May 12, 2015, 1:43 p.m.) Review request for mesos. Changes ---

Re: Review Request 33241: docs: Add documentation on observability metrics.

> On April 30, 2015, 4:09 p.m., Joe Smith wrote: > > docs/metrics.md, line 221 > > > > > > master/slave_shutdowns_completed > > > > These are slaves which were not heard from despite the slave-removal > > rat

Re: Review Request 33875: Removed unused collect cycle in resource monitor

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33875/ --- (Updated May 12, 2015, 1:54 p.m.) Review request for mesos, Jie Yu and Vinod Ko

Re: Review Request 33876: Added usages() to resource monitor

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33876/ --- (Updated May 12, 2015, 1:55 p.m.) Review request for mesos, Jie Yu and Vinod Ko

Re: Review Request 33876: Added usages() to resource monitor

> On May 11, 2015, 3:49 p.m., Jie Yu wrote: > > src/slave/monitor.cpp, line 289 > > > > > > The prefix `ResourceMonitor::` is not needed here This one is needed, to compile > On May 11, 2015, 3:49 p.m., Jie Yu wrote

Re: Review Request 33275: Fix capture by reference of temporaries in Stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33275/#review83478 --- I found 2 instances which look like they were missed: `check.hpp` `

Re: Review Request 33272: Fix capture by reference of temporary strings in Stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33272/#review83480 --- Ship it! Ship It! - Michael Park On April 22, 2015, 6:11 p.m., J

Re: Review Request 30609: Added a function that reports file size, not following links.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30609/ --- (Updated May 12, 2015, 2:23 p.m.) Review request for mesos, Benjamin Hindman, T

Re: Review Request 33274: Fix capture by reference of temporary strings in Libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33274/#review83488 --- Looks good overall! I think found one more: `src/process.cpp` ```c

Re: Review Request 33241: docs: Add documentation on observability metrics.

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

Re: Review Request 33276: Fix capture by reference of temporaries in Libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33276/#review83495 --- Again, looks great overall! I think these need to be updated as wel

Re: Review Request 30609: Added a function that reports file size, not following links.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30609/#review83497 --- Ship it! Ship It! - Timothy Chen On May 12, 2015, 9:23 p.m., Ber

Re: Review Request 33876: Added usages() to resource monitor

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

Re: Review Request 33152: Moved the slave shutdown test into slave_tests.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33152/ --- (Updated May 12, 2015, 10:22 p.m.) Review request for mesos and Vinod Kone. R

Re: Review Request 30774: Fetcher Cache

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30774/ --- (Updated May 12, 2015, 3:43 p.m.) Review request for mesos, Adam B, Benjamin Hi

Review Request 34128: Enable different IP/Port for external access.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34128/ --- Review request for mesos. Bugs: MESOS-809 https://issues.apache.org/jira/br

Re: Review Request 34128: Enable different IP/Port for external access.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34128/ --- (Updated May 12, 2015, 11:23 p.m.) Review request for mesos. Bugs: MESOS-809

Review Request 34129: Add 2 optional args public_ip and public_port for libprocess to advertise.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/ --- Review request for mesos. Bugs: MESOS-809 https://issues.apache.org/jira/br

Re: Review Request 33153: Moved a partition test into partition_tests.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33153/ --- (Updated May 12, 2015, 11:28 p.m.) Review request for mesos and Vinod Kone. C

Re: Review Request 33154: Added reason metrics for slave removals.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33154/ --- (Updated May 12, 2015, 11:28 p.m.) Review request for mesos and Vinod Kone. C

Re: Review Request 33154: Added reason metrics for slave removals.

> On May 12, 2015, 7:31 p.m., Vinod Kone wrote: > > LGTM. Can you add a comment on removeSlave() on why it takes a const ref of > > 'reason' per Alex's comments? I've just removed the const reference instead, to take a copy. - Ben ---

Re: Review Request 33155: Added commented-out tests for slave removal metrics.

> On April 16, 2015, 11:04 p.m., Vinod Kone wrote: > > src/tests/master_tests.cpp, lines 1725-1731 > > > > > > I'm a bit confused on how slave's lifetime affects the metrics > > collection on master endpoint? > > Be

Re: Review Request 33155: Added tests for slave removal metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33155/ --- (Updated May 12, 2015, 11:32 p.m.) Review request for mesos and Vinod Kone. C

Re: Review Request 34129: Add 2 optional args public_ip and public_port for libprocess to advertise.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34129/#review83524 --- You should update this help message for the http master/redirect: ht

Re: Review Request 34128: Enable different IP/Port for external access.

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

Re: Review Request 32891: Support for entering and configuring a Linux chroot.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32891/ --- (Updated May 12, 2015, 5:22 p.m.) Review request for mesos, Chi Zhang, Jay Buff

Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31444/ --- (Updated May 12, 2015, 5:22 p.m.) Review request for mesos, Chi Zhang, Dominic

Re: Review Request 30643: Optionally specify executor for "mesos execute".

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30643/ --- (Updated May 12, 2015, 5:22 p.m.) Review request for mesos, Timothy Chen and Vi

Re: Review Request 33918: Added resources estimator abstraction for oversubscription.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33918/ --- (Updated May 13, 2015, 12:23 a.m.) Review request for mesos, Ben Mahler, Joris

Re: Review Request 33746: Improve logging of clone flags for linux_launcher.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33746/ --- (Updated May 12, 2015, 5:23 p.m.) Review request for mesos, Chi Zhang and Jie Y

Re: Review Request 34128: Enable different IP/Port for external access.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34128/#review83533 --- 3rdparty/libprocess/src/process.cpp

Re: Review Request 33919: Integrated resources estimator with the slave.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33919/ --- (Updated May 13, 2015, 12:37 a.m.) Review request for mesos, Ben Mahler, Joris

Re: Review Request 33919: Integrated resources estimator with the slave.

> On May 7, 2015, 6:16 p.m., Niklas Nielsen wrote: > > src/messages/messages.proto, line 339 > > > > > > Is the slave id going to be implicit from the sender PID in the master? > > Also, do we need a time stamp? Y

Review Request 34134: Add container rootfs to Isolator::prepare().

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34134/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Review Request 34135: Add filesystem/ isolators for persistent volumes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34135/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Review Request 34137: Add support for container image provisioners.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34137/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Review Request 34138: AppC hash computation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34138/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Review Request 34136: Add ContainerImage protobuf.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34136/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Review Request 34140: Appc image store

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34140/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Review Request 34142: AppC provisioner.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34142/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Review Request 34139: AppC image discovery.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34139/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Review Request 34141: AppC provsioning backend.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34141/ --- Review request for mesos, Chi Zhang, Paul Brett, Timothy Chen, and Vinod Kone.

Re: Review Request 33155: Added tests for slave removal metrics.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33155/#review83538 --- Patch looks great! Reviews applied: [33152, 33153, 33154, 33155] A

Re: Review Request 34129: Add 2 optional args public_ip and public_port for libprocess to advertise.

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

Review Request 34152: Master flag validation now supports zookeeper

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34152/ --- Review request for mesos. Bugs: MESOS-2723 https://issues.apache.org/jira/b

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

> On May 11, 2015, 10:19 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 155 > > > > > > unique_ptr is not a POD, so this will still try to run the destructor > > of the function during exit of the program. > >

Re: Review Request 34016: Change the type of signaledWrapper to unique_ptr

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34016/ --- (Updated May 13, 2015, 2:21 a.m.) Review request for mesos and Ben Mahler. Re

Re: Review Request 30643: Optionally specify executor for "mesos execute".

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

Re: Review Request 31444: Support chrooting in MesosContainerizer launch helper.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31444/#review83553 --- Patch looks great! Reviews applied: [32891, 32978, 31444] All test

Re: Review Request 33746: Improve logging of clone flags for linux_launcher.

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

Re: Review Request 34138: AppC hash computation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34138/#review83557 --- src/slave/containerizer/provisioners/appc/hash.hpp

Re: Review Request 34135: Add filesystem/ isolators for persistent volumes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34135/#review83558 --- src/slave/containerizer/isolators/filesystem/linux.cpp

Re: Review Request 33919: Integrated resources estimator with the slave.

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

Re: Review Request 34142: AppC provisioner.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34142/#review83564 --- Bad patch! Reviews applied: [34134, 34135, 34134] Failed command:

Re: Review Request 34152: Master flag validation now supports zookeeper

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