Re: Review Request 28697: [WIP] Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-29 Thread Michael Park
/28697/#review65519 --- On Dec. 17, 2014, 12:56 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28697

Re: Review Request 28697: Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-29 Thread Michael Park
necessary to support dynamic reservations. Diffs - include/mesos/mesos.proto 540071db64961466eb75c779b3ea6863f4594437 Diff: https://reviews.apache.org/r/28697/diff/ Testing --- make check Thanks, Michael Park

Re: Review Request 28697: Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-29 Thread Michael Park
540071db64961466eb75c779b3ea6863f4594437 Diff: https://reviews.apache.org/r/28697/diff/ Testing --- make check Thanks, Michael Park

Re: Review Request 28697: Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-29 Thread Michael Park
--- On Dec. 30, 2014, 2:42 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28697

Re: Review Request 28698: [WIP] Modified Resources to account for reservation type.

2014-12-29 Thread Michael Park
c17e1791130e7d545bb7cdd54d97d65325d3a69e Diff: https://reviews.apache.org/r/28698/diff/ Testing --- make check Thanks, Michael Park

Re: Review Request 28697: Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-29 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28697/#review65670 --- On Dec. 30, 2014, 2:42 a.m., Michael Park wrote

Re: Review Request 28697: Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-29 Thread Michael Park
by what kind of message (acquire/release/launch) this Resource is a part of. That said, I am all the more convinced that we should name the enum ReservationType to prepare for a day when we need a fuller Reservation message. Michael Park wrote: Removed WIP. Changed Reservation

Re: Review Request 29233: Add instruction for ClangFormat installation and editor integration.

2014-12-23 Thread Michael Park
://reviews.apache.org/r/29233/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28720: Adjusted the calculation of unused resources in _launchTasks by considering persistent disk acquisition.

2014-12-18 Thread Michael Park
On Dec. 18, 2014, 2:31 a.m., Ben Mahler wrote: src/master/master.cpp, lines 1897-1902 https://reviews.apache.org/r/28720/diff/7/?file=795193#file795193line1897 Ah, we should consider how to make these unit testable, since ResourceValidator doesn't actually need Framework and

Review Request 29233: Add instruction for ClangFormat installation and editor integration.

2014-12-18 Thread Michael Park
, Michael Park

Review Request 29173: [WIP] Add dynamic reservation information to LaunchTasksMessage.

2014-12-17 Thread Michael Park
, and Vinod Kone. Repository: mesos-git Description --- Add dynamic reservation information to LaunchTasksMessage. Diffs - src/messages/messages.proto 6b261f7815fcc8ff80fc49d4804ecb72614a14f3 Diff: https://reviews.apache.org/r/29173/diff/ Testing --- Thanks, Michael Park

Re: Review Request 29173: [WIP] Add dynamic reservation information to LaunchTasksMessage.

2014-12-17 Thread Michael Park
6b261f7815fcc8ff80fc49d4804ecb72614a14f3 Diff: https://reviews.apache.org/r/29173/diff/ Testing --- Thanks, Michael Park

Review Request 29174: [WIP] Update the master and allocator + Added a test for acquiring dynamic reservations.

2014-12-17 Thread Michael Park
: https://reviews.apache.org/r/29174/diff/ Testing --- Thanks, Michael Park

Re: Review Request 29174: [WIP] Update the master and allocator + Added a test for acquiring dynamic reservations.

2014-12-17 Thread Michael Park
/ instead. src/master/master.cpp https://reviews.apache.org/r/29174/#comment108468 Still need to fill in `TODO`s such as validation. - Michael Park On Dec. 17, 2014, 7:40 p.m., Michael Park wrote: --- This is an automatically

Re: Review Request 29173: [WIP] Add dynamic reservation information to LaunchTasksMessage.

2014-12-17 Thread Michael Park
/#comment108469 Perhaps for dynamic reservations I should move away from *release/acquire* and use *unreserve/reserve* instead? - Michael Park On Dec. 17, 2014, 7:38 p.m., Michael Park wrote: --- This is an automatically generated e

Review Request 29175: [WIP] Update the C++ Scheduler API.

2014-12-17 Thread Michael Park
4be08f12e126ac192a5247ec426a36610bb021d1 Diff: https://reviews.apache.org/r/29175/diff/ Testing --- Thanks, Michael Park

Re: Review Request 29175: [WIP] Update the C++ Scheduler API.

2014-12-17 Thread Michael Park
/scheduler.hpp 42e4e279d059801cd85955fd04995b60051a2b5e src/sched/sched.cpp 4be08f12e126ac192a5247ec426a36610bb021d1 Diff: https://reviews.apache.org/r/29175/diff/ Testing --- Thanks, Michael Park

Re: Review Request 29174: [WIP] Update the master and allocator + Added a test for acquiring dynamic reservations.

2014-12-17 Thread Michael Park
bb24222c20cb5458b5c627d2001fc3cb1e542cce Diff: https://reviews.apache.org/r/29174/diff/ Testing --- Thanks, Michael Park

Re: Failure to build (possibly a 3rd party issue)

2014-12-17 Thread Michael Park
Hi Ritwik, I've seen this problem before but haven't had time to look into it. We made C++11 a requirement so the Makefile actually isn't the issue here. I can look into what's actually causing this, Thanks for bringing it up. MPark On 17 December 2014 at 07:44, Ritwik ritwik.ya...@gmail.com

Re: Failure to build (possibly a 3rd party issue)

2014-12-17 Thread Michael Park
Hi Ritwik, I've seen this problem before but haven't had time to look into it. We made C++11 a requirement so the Makefile actually isn't the issue here. I can look into it though, Thanks for bringing it up! MPark On 17 December 2014 at 07:44, Ritwik ritwik.ya...@gmail.com wrote: I dug

Review Request 29180: Fixed a compilation issue in GCC 4.6.

2014-12-17 Thread Michael Park
: https://reviews.apache.org/r/29180/diff/ Testing --- make check Thanks, Michael Park

Re: Review Request 29180: Fixed a compilation issue in GCC 4.6.

2014-12-17 Thread Michael Park
/libprocess/src/clock.cpp dfe9ced2415b8567abb8c137ab73d90b59164d67 3rdparty/libprocess/src/process.cpp 028b33e7ecb7e0a39334ac4ab0279ee327a72a56 Diff: https://reviews.apache.org/r/29180/diff/ Testing --- make check Thanks, Michael Park

Re: Review Request 29175: [WIP] Update the C++ Scheduler API.

2014-12-17 Thread Michael Park
/scheduler.hpp 42e4e279d059801cd85955fd04995b60051a2b5e src/sched/sched.cpp 4be08f12e126ac192a5247ec426a36610bb021d1 Diff: https://reviews.apache.org/r/29175/diff/ Testing --- Thanks, Michael Park

Re: Review Request 29174: [WIP] Update the master and allocator + Added a test for acquiring dynamic reservations.

2014-12-17 Thread Michael Park
bb24222c20cb5458b5c627d2001fc3cb1e542cce Diff: https://reviews.apache.org/r/29174/diff/ Testing --- Thanks, Michael Park

Re: Review Request 29180: Fixed a compilation issue in GCC 4.6.

2014-12-17 Thread Michael Park
dfe9ced2415b8567abb8c137ab73d90b59164d67 3rdparty/libprocess/src/process.cpp 028b33e7ecb7e0a39334ac4ab0279ee327a72a56 Diff: https://reviews.apache.org/r/29180/diff/ Testing --- make check Thanks, Michael Park

Re: Review Request 29180: Fixed a compilation issue in GCC 4.6.

2014-12-17 Thread Michael Park
); } ``` - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29180/#review65386 --- On Dec. 17, 2014, 11:33 p.m., Michael Park wrote

Re: Review Request 29180: Fixed a compilation issue in GCC 4.6.

2014-12-17 Thread Michael Park
On Dec. 17, 2014, 11:36 p.m., Dominic Hamon wrote: any idea what is missing from g++ 4.6 that causes this failure? Michael Park wrote: It's a bug in the `tr1/functional` header. The `operator()`'s implementation doesn't perfect-forward the args correctly. ```cpp

Re: Failure to build (possibly a 3rd party issue)

2014-12-17 Thread Michael Park
, not knowing what caused the problem might prove to be bad in the long run. Thanks again for all the help. On 18 December 2014 at 01:40, Michael Park mcyp...@gmail.com wrote: Hi Ritwik, I've seen this problem before but haven't had time to look into it. We made C++11 a requirement so

Re: Review Request 28697: [WIP] Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-16 Thread Michael Park
messages necessary to support dynamic reservations. Work in progress. Diffs - include/mesos/mesos.proto e0b6375579ca7d3af03e4b59044810b8b7d2df86 Diff: https://reviews.apache.org/r/28697/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28698: [WIP] Modified Resources to account for reservation type.

2014-12-16 Thread Michael Park
- include/mesos/resources.hpp 10777a62492e4ad764e0f75c064694e054d1 src/common/resources.cpp 535a0eab6377b9ae63c960cdb05978647f667d5e Diff: https://reviews.apache.org/r/28698/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28697: [WIP] Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-16 Thread Michael Park
540071db64961466eb75c779b3ea6863f4594437 Diff: https://reviews.apache.org/r/28697/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28698: [WIP] Modified Resources to account for reservation type.

2014-12-16 Thread Michael Park
9bf7ae9148d6db2829cc2866ac048fe018ae2c92 Diff: https://reviews.apache.org/r/28698/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28698: [WIP] Modified Resources to account for reservation type.

2014-12-16 Thread Michael Park
Diff: https://reviews.apache.org/r/28698/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28698: [WIP] Modified Resources to account for reservation type.

2014-12-16 Thread Michael Park
believe it's a better API but I'm wondering if it has backwards-compatibility issues. - Michael Park On Dec. 16, 2014, 11:04 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 28698: [WIP] Modified Resources to account for reservation type.

2014-12-16 Thread Michael Park
Diff: https://reviews.apache.org/r/28698/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28697: [WIP] Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-16 Thread Michael Park
540071db64961466eb75c779b3ea6863f4594437 Diff: https://reviews.apache.org/r/28697/diff/ Testing (updated) --- make check Thanks, Michael Park

Re: Review Request 28698: [WIP] Modified Resources to account for reservation type.

2014-12-16 Thread Michael Park
Diff: https://reviews.apache.org/r/28698/diff/ Testing (updated) --- make check Thanks, Michael Park

Re: Review Request 29018: Added abstraction for resources transformation.

2014-12-15 Thread Michael Park
On Dec. 15, 2014, 6:08 p.m., Dominic Hamon wrote: include/mesos/resources.hpp, line 213 https://reviews.apache.org/r/29018/diff/1/?file=791218#file791218line213 transformations.emplace_back (if we have it in 4.4) might save you the explicit move call, and will certainly save you

Re: Review Request 29018: Added abstraction for resources transformation.

2014-12-15 Thread Michael Park
On Dec. 15, 2014, 9:47 p.m., Dominic Hamon wrote: include/mesos/resources.hpp, line 213 https://reviews.apache.org/r/29018/diff/1/?file=791218#file791218line213 not quite. emplace_back with a unique_ptr works because unique_ptr doesn't have a copy constructor. however, as it

Re: Review Request 28722: Modify libprocess configure.ac to set $with_svn to `brew --prefix subversion` for OS X. Fixes MESOS-2113.

2014-12-05 Thread Michael Park
314c8b842d07e1344659449866f7e85c48854204 Diff: https://reviews.apache.org/r/28722/diff/ Testing --- `../configure make make check` on Ubuntu and OS X Thanks, Michael Park

Re: Review Request 28723: Added protobuf protocol for communicating persisted resources between master and slave.

2014-12-05 Thread Michael Park
in order to stay clear of confusing it with persistent resources. - Michael Park On Dec. 4, 2014, 10:56 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28723

Re: Review Request 28721: Modify mesos configure.ac to set $with_svn to `brew --prefix subversion` for OS X. Fixes MESOS-2113.

2014-12-05 Thread Michael Park
1ca6a6b3aa391275dda1b00de27e212c4fc8190f Diff: https://reviews.apache.org/r/28721/diff/ Testing --- `../configure make make check` on Ubuntu and OS X Thanks, Michael Park

Re: Review Request 28722: Modify libprocess configure.ac to set $with_svn to `brew --prefix subversion` for OS X. Fixes MESOS-2113.

2014-12-05 Thread Michael Park
314c8b842d07e1344659449866f7e85c48854204 Diff: https://reviews.apache.org/r/28722/diff/ Testing --- `../configure make make check` on Ubuntu and OS X Thanks, Michael Park

Re: Review Request 28721: Modify mesos configure.ac to set $with_svn to `brew --prefix subversion` for OS X. Fixes MESOS-2113.

2014-12-05 Thread Michael Park
1ca6a6b3aa391275dda1b00de27e212c4fc8190f Diff: https://reviews.apache.org/r/28721/diff/ Testing --- `../configure make make check` on Ubuntu and OS X Thanks, Michael Park

Re: Review Request 28697: Add ReservationType and ReservationInfo for dynamic reservations.

2014-12-05 Thread Michael Park
e0b6375579ca7d3af03e4b59044810b8b7d2df86 Diff: https://reviews.apache.org/r/28697/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28698: Modified Resources arithmetic logic to account for reservation type.

2014-12-05 Thread Michael Park
535a0eab6377b9ae63c960cdb05978647f667d5e Diff: https://reviews.apache.org/r/28698/diff/ Testing --- Thanks, Michael Park

Re: Review Request 28052: Match future dispatch messages with type info.

2014-12-04 Thread Michael Park
of and the offset within the vtable. The `typeid` check gets us the former and the previous function pointer check gets us the latter. - Michael Park On Nov. 20, 2014, 9 a.m., Timothy Chen wrote: --- This is an automatically generated e-mail

Re: Review Request 28721: Modify mesos configure.ac to set $with_svn to `brew --prefix subversion` for OS X. Fixes MESOS-2113.

2014-12-04 Thread Michael Park
: https://reviews.apache.org/r/28721/diff/ Testing --- `../configure make make check` on Ubuntu and OS X Thanks, Michael Park

Re: Review Request 28309: Install clang-format on bootstrap

2014-11-20 Thread Michael Park
that mean https://github.com/apache/mesos/blob/master/.clang-format is somehow missing? - Michael Park On Nov. 21, 2014, 1:18 a.m., Cody Maloney wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 28309: Install clang-format on bootstrap

2014-11-20 Thread Michael Park
On Nov. 21, 2014, 1:41 a.m., Michael Park wrote: Where is it installing symlink? If it's the Mesos src dir, does that mean https://github.com/apache/mesos/blob/master/.clang-format is somehow missing? Cody Maloney wrote: I thought I had put that symlink there myself, guess we

Re: Review Request 28089: Refactored operators for Resource object and made them private.

2014-11-17 Thread Michael Park
On Nov. 15, 2014, 11 p.m., Michael Park wrote: include/mesos/resources.hpp, lines 102-106 https://reviews.apache.org/r/28089/diff/1/?file=764758#file764758line102 I would suggest keeping the behavior symmetric to `std::vector` here. I'm guessing this was added due

Re: Review Request 28091: Always store validated and combined Resource objects in C++ Resources.

2014-11-17 Thread Michael Park
On Nov. 16, 2014, 1:10 a.m., Michael Park wrote: src/common/resources.cpp, line 391 https://reviews.apache.org/r/28091/diff/2/?file=765012#file765012line391 +1 Ben Mahler wrote: Are you +1'ing my comments? If so, to keep threads intact you have to comment on the main

Re: Review Request 28052: Match future dispatch messages with type info.

2014-11-15 Thread Michael Park
On Nov. 15, 2014, 4:15 a.m., Michael Park wrote: Suggestion: We can simply return `const std::type_info*` rather than `std::string` for `canonicalize`. The implementation would stay pretty much the same, and we wouldn't have to construct these `MatchesTypeFunc` instances

Re: Review Request 28092: Removed public operators for repeated Resource protobuf.

2014-11-15 Thread Michael Park
On Nov. 15, 2014, 7:22 p.m., Ben Mahler wrote: include/mesos/resources.hpp, lines 184-189 https://reviews.apache.org/r/28092/diff/1/?file=765009#file765009line184 Just curious, did we even need this in the first place? Looks like implicit conversion will result in us using the

Re: Review Request 28092: Removed public operators for repeated Resource protobuf.

2014-11-15 Thread Michael Park
`validate` and/or `flatten` explicitly as well? - Michael Park On Nov. 15, 2014, 7:11 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28092

Re: Review Request 28093: Replaced size() with empty() in C++ Resources.

2014-11-15 Thread Michael Park
()` rather than `size() == 0` and `!empty()` rather than `size() 0`. But do you think we should get rid of `size()`? It seems like a reasonable thing to be able to do on a list of resources. - Michael Park On Nov. 15, 2014, 7:24 a.m., Jie Yu wrote

Re: Review Request 28078: Re-organized the functions in C++ Resources.

2014-11-15 Thread Michael Park
On Nov. 15, 2014, 7:17 p.m., Ben Mahler wrote: include/mesos/resources.hpp, lines 312-317 https://reviews.apache.org/r/28078/diff/2/?file=764753#file764753line312 Could we get away with only needing iosfwd if we move this into the .cpp file? I think iostream is pretty

Re: Review Request 28088: A few style fixes for C++ Resources and tests.

2014-11-15 Thread Michael Park
On Nov. 15, 2014, 7:59 p.m., Ben Mahler wrote: Modulo minor comments. +1 to all 3 - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28088/#review61659

Re: Review Request 28094: Replaced = with contains() in C++ Resources.

2014-11-15 Thread Michael Park
need to be rebased? In the up-to-date master, `OptionResources find(const Resources targets) const;` has changed to `OptionResources find(const Resources toFind, const std::string role = *) const;`. I think the definition also changed enough to affect this patch. - Michael Park On Nov. 15, 2014

Re: Review Request 28090: Killed ports allocation in C++ Resources.

2014-11-15 Thread Michael Park
that. - Michael Park On Nov. 15, 2014, 7:01 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28090/ --- (Updated Nov

Re: Review Request 28089: Refactored operators for Resource object and made them private.

2014-11-15 Thread Michael Park
` here even if we're now doing `CopyFrom`? - Michael Park On Nov. 15, 2014, 6:56 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28089

Re: Review Request 28094: Replaced = with contains() in C++ Resources.

2014-11-15 Thread Michael Park
On Nov. 15, 2014, 9:44 p.m., Michael Park wrote: Looks good overall to me. But it looks like these patches might need to be rebased? In the up-to-date master, `OptionResources find(const Resources targets) const;` has changed to `OptionResources find(const Resources toFind, const

Re: Review Request 28089: Refactored operators for Resource object and made them private.

2014-11-15 Thread Michael Park
On Nov. 15, 2014, 11:15 p.m., Michael Park wrote: src/common/resources.cpp, lines 800-804 https://reviews.apache.org/r/28089/diff/1/?file=764759#file764759line800 I know you didn't touch this part, but could you make it so that `Resources Resources::operator + (const Resource

Re: Review Request 28094: Replaced = with contains() in C++ Resources.

2014-11-15 Thread Michael Park
), this, lambda::_1)); ``` Sadness. - Michael Park On Nov. 15, 2014, 7:32 a.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28094

Re: Review Request 28091: Always store validated and combined Resource objects in C++ Resources.

2014-11-15 Thread Michael Park
, that, lambda::_1)); if (iter != std::end(resources)) { *iter += that; } else { resources.Add()-CopyFrom(that); } ``` - Michael Park On Nov. 15, 2014, 7:18 a.m., Jie Yu wrote: --- This is an automatically

Re: Review Request 28081: Fix future dispatch in tests to match function type.

2014-11-14 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28081/#review61623 --- LGTM - Michael Park On Nov. 15, 2014, 1:54 a.m., Timothy Chen

Re: Review Request 28052: Match future dispatch messages with type info.

2014-11-14 Thread Michael Park
to be resolved at compile-time unless it's invoked on a dynamic type. In this case we're only calling it on member function pointers, which means it'll be resolved at compile-time and therefore won't ever incur any RTTI costs. - Michael Park On Nov. 15, 2014, 1:53 a.m., Timothy Chen wrote

Dynamic Reservations Design Doc

2014-11-10 Thread Michael Park
Hello all, I'd like to share the design doc for dynamic reservations https://docs.google.com/document/d/1e3j69pfBgtc8xM00DhcuiMl6ImkEB5na0TzOMyzrg8A/edit?usp=sharing, a feature to better support stateful services. Dynamic reservations allow a framework to dynamically reserve offered resources,

Re: Review Request 27446: libprocess: Replaced the ip and port pairs from UPID class and process namespace with Node class.

2014-11-10 Thread Michael Park
://reviews.apache.org/r/27446/#comment102101 Let's just replace the 3 references to `node` to `to.node` rather than copying here. - Michael Park On Nov. 7, 2014, 5:34 p.m., Evelina Dumitrescu wrote: --- This is an automatically generated

Re: Review Request 27446: libprocess: Replaced the ip and port pairs from UPID class and process namespace with Node class.

2014-11-10 Thread Michael Park
On Nov. 11, 2014, 1:59 a.m., Michael Park wrote: Looks good, fairly straigth forward transformation :) - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27446/#review60720

Re: Review Request 27446: libprocess: Replaced the ip and port pairs from UPID class and process namespace with Node class.

2014-11-10 Thread Michael Park
On Nov. 11, 2014, 1:59 a.m., Michael Park wrote: Michael Park wrote: Looks good, fairly straigth forward transformation :) s/straigth/straight/ - Michael --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 27447: mesos: Replaced the ip and port pairs from UPID class and process namespace with Node class.

2014-11-10 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27447/#review60739 --- +1 looks good - Michael Park On Nov. 7, 2014, 3:23 a.m., Evelina

Re: Review Request 25735: change const pass-by-value to const reference in stout

2014-10-29 Thread Michael Park
is undefined.__ - Michael Park On Sept. 17, 2014, 2:25 p.m., Kamil Domanski wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25735

Re: Review Request 26533: Memory cleanup: libprocess finalize

2014-10-10 Thread Michael Park
On Oct. 10, 2014, 6:02 p.m., Jie Yu wrote: 3rdparty/libprocess/src/process.cpp, line 2449 https://reviews.apache.org/r/26533/diff/1/?file=717191#file717191line2449 We usually prefer an explict representation: ``` if (ptr != nullptr) { } ```

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

2014-10-09 Thread Michael Park
/26436/#comment96314 Can we make the container here `const`? We don't actually do any modification on it in this function. - Michael Park On Oct. 8, 2014, 6:43 p.m., Timothy Chen wrote: --- This is an automatically generated e-mail

Re: Review Request 25848: Introducing mesos modules.

2014-10-08 Thread Michael Park
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 [MESOS-1023](https

Re: Reserved names

2014-10-08 Thread Michael Park
, naming things, and off-by-one errors. On Sun, Oct 5, 2014 at 4:20 PM, Michael Park mcyp...@gmail.com wrote: Hello, I just wanted to mention an issue with our naming convention that goes against the C++ standard due to the rules around reserved names. From N3797, 17.6.4.3

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

2014-10-08 Thread Michael Park
? src/slave/containerizer/docker.cpp https://reviews.apache.org/r/26436/#comment96167 Are these checks necessary? In both calls to `__usage` directly rather than through `defer` or something similar. - Michael Park On Oct. 8, 2014, 4:51 a.m., Timothy Chen wrote

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

2014-10-08 Thread Michael Park
is replaced, I mark this issue as fixed, please reopen if you think further discussion about `return` is needed. Michael Park wrote: My thoughts on the two purposes: 1. I actually think a comment would do serve the purpose better in terms of being clear on what we're

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

2014-10-08 Thread Michael Park
is replaced, I mark this issue as fixed, please reopen if you think further discussion about `return` is needed. Michael Park wrote: My thoughts on the two purposes: 1. I actually think a comment would do serve the purpose better in terms of being clear on what we're

Re: Review Request 26150: Libprocess Benchmark

2014-10-08 Thread Michael Park
On Oct. 8, 2014, 5:32 p.m., Niklas Nielsen wrote: 3rdparty/libprocess/src/tests/benchmarks.cpp, line 200 https://reviews.apache.org/r/26150/diff/1/?file=708531#file708531line200 I am not sure we have graced range based loops yet. Do you have any references to whether it is

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
/#comment96031 It looks like `taskMap.erase(taskId)` needs to modify the `framework-pending` hashmap. I think we want `foreachvalue (TaskMap taskMap, framework-pending) { ... }` here? - Michael Park On Oct. 7, 2014, 3:18 p.m., Bernd Mathiske wrote

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 25848: Introducing mesos modules.

2014-10-07 Thread Michael Park
= PTHREAD_MUTEX_INITIALIZER; return singleton; } ``` - Michael Park On Oct. 7, 2014, 11:07 p.m., Kapil Arya wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25848

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 26352: Add --with-curl to libprocess since stout/net.hpp needs it.

2014-10-06 Thread Michael Park
://reviews.apache.org/r/26352/diff/ Testing --- Built `mesos` on a machine without system-level libcurl installed with `--with-curl` flag. Thanks, Michael Park

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

2014-10-06 Thread Michael Park
://reviews.apache.org/r/26352/diff/ Testing --- Built `mesos` on a machine without system-level libcurl installed with `--with-curl` flag. Thanks, Michael Park

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

2014-10-06 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26352/#review55495 --- On Oct. 6, 2014, 1:50 p.m., Michael Park wrote

Re: Review Request 26247: Remove usage of stout/preprocessor.hpp from mesos.

2014-10-05 Thread Michael Park
/zookeeper.cpp d4c24cd500b74d3b979a471b4a32def78958f04a Diff: https://reviews.apache.org/r/26247/diff/ Testing --- `make make check` on `gcc-4.4`, `gcc-4.6`, `gcc-4.8`, `gcc-4.9`, `clang-3.3` and `clang-3.5` Thanks, Michael Park

Re: Review Request 26246: Remove usage of stout/preprocessor.hpp from libprocess.

2014-10-05 Thread Michael Park
`, `gcc-4.9`, `clang-3.3` and `clang-3.5` Thanks, Michael Park

Re: Review Request 26247: Remove usage of stout/preprocessor.hpp from mesos.

2014-10-05 Thread Michael Park
: https://reviews.apache.org/r/26247/diff/ Testing --- `make make check` on `gcc-4.4`, `gcc-4.6`, `gcc-4.8`, `gcc-4.9`, `clang-3.3` and `clang-3.5` Thanks, Michael Park

Re: Review Request 26247: Remove usage of stout/preprocessor.hpp from mesos.

2014-10-05 Thread Michael Park
. To reply, visit: https://reviews.apache.org/r/26247/#review55140 --- On Oct. 5, 2014, 6:05 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 26245: Remove usage of stout/preprocessor.hpp from stout.

2014-10-05 Thread Michael Park
/#review55130 --- On Oct. 1, 2014, 9:18 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26245

Re: Review Request 26246: Remove usage of stout/preprocessor.hpp from libprocess.

2014-10-05 Thread Michael Park
make check` on `gcc-4.4`, `gcc-4.6`, `gcc-4.8`, `gcc-4.9`, `clang-3.3` and `clang-3.5` Thanks, Michael Park

Re: Review Request 26245: Remove usage of stout/preprocessor.hpp from stout.

2014-10-05 Thread Michael Park
`, `gcc-4.8`, `gcc-4.9`, `clang-3.3` and `clang-3.5` Thanks, Michael Park

Re: Review Request 26247: Remove usage of stout/preprocessor.hpp from mesos.

2014-10-05 Thread Michael Park
` Thanks, Michael Park

Re: Review Request 26245: Remove usage of stout/preprocessor.hpp from stout.

2014-10-05 Thread Michael Park
--- On Oct. 5, 2014, 6:57 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26245/ --- (Updated Oct. 5, 2014, 6

Re: Review Request 26069: Introduce ClangFormat to Mesos.

2014-10-05 Thread Michael Park
to the __Sample Diff__ section in the [Google Doc](https://docs.google.com/document/d/13mC3CuG89x0-4mGUD1NK-M0mYsqvEcZ-ttx9CRmAXq8/edit?usp=sharing) Thanks, Michael Park

Re: Review Request 26246: Remove usage of stout/preprocessor.hpp from libprocess.

2014-10-05 Thread Michael Park
fd906b869e8bbf5c07c67c5319dec82681e811b7 Diff: https://reviews.apache.org/r/26246/diff/ Testing --- `make make check` on `gcc-4.4`, `gcc-4.6`, `gcc-4.8`, `gcc-4.9`, `clang-3.3` and `clang-3.5` Thanks, Michael Park

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

2014-10-05 Thread Michael Park
/diff/ Testing (updated) --- Built `mesos` on a machine without system-level libcurl installed with `--with-curl` flag. Thanks, Michael Park

<    3   4   5   6   7   8   9   >