Re: Review Request 32996: Updated roadmap document to link to 'Epic' tickets.

2015-04-23 Thread Ben Mahler
--- On April 9, 2015, 12:30 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32996

Re: Review Request 32998: Split committer's guide into code reviewing and committing documents.

2015-04-23 Thread Ben Mahler
: https://reviews.apache.org/r/32998/#review80786 --- On April 9, 2015, 12:30 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 32998: Split committer's guide into code reviewing and committing documents.

2015-04-23 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32998/#review79457 --- On April 9, 2015, 12:30 a.m., Ben Mahler wrote

Re: Review Request 33358: Moved implementation of StatusUpdateStream to a compilation unit.

2015-04-22 Thread Ben Mahler
On April 22, 2015, 9:24 a.m., Alexander Rukletsov wrote: Let's not repeat summary in description. How about the following: StatusUpdateStream is not a template, hence we can reduce compilation time by moving method definitions into a `.cpp` file. As a drive-by change headers are

Re: Review Request 32844: Added SUBSCRIBE call and SUBSCRIBED event.

2015-04-22 Thread Ben Mahler
https://reviews.apache.org/r/32844/#comment131478 Feel free to tease this change out and commit on its own. No need for review, just might be nice to have a separate bug fix commit? - Ben Mahler On April 22, 2015, 9:42 p.m., Vinod Kone wrote

Re: Review Request 32509: Documented the scheduler Event/Call protobufs.

2015-04-22 Thread Ben Mahler
/32509/#comment131519 Hm.. schedulers can't use Update as a replacement for this Message, since Message is scheduler - executor but Update is executor - scheduler.. - Ben Mahler On April 22, 2015, 9:44 p.m., Vinod Kone wrote

Re: Review Request 32843: Updated KILL to include SlaveID.

2015-04-22 Thread Ben Mahler
/tests/scheduler_tests.cpp https://reviews.apache.org/r/32843/#comment131476 Did you want this extra newline? - Ben Mahler On April 22, 2015, 9:40 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 32845: Renamed UNREGISTER call to UNSUBSCRIBE.

2015-04-22 Thread Ben Mahler
/ here? Could you do a grep just in case anything else was missed? src/tests/scheduler_tests.cpp https://reviews.apache.org/r/32845/#comment131492 Hm.. can you add a comment above this as to why it's needed? - Ben Mahler On April 22, 2015, 9:43 p.m., Vinod Kone wrote

Re: Review Request 33491: Set the supplementary groups list when switching users.

2015-04-24 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33491/#review81534 --- Ship it! Ship It! - Ben Mahler On April 23, 2015, 8:16 p.m

Re: Review Request 32998: Split committer's guide into code reviewing and committing documents.

2015-04-23 Thread Ben Mahler
--- On April 9, 2015, 12:30 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32998

Re: Review Request 32999: Added a document for engineering principles and practices.

2015-04-23 Thread Ben Mahler
641ee8f5e7cc2f9ccd62a5c4236912886aaa7a1d Diff: https://reviews.apache.org/r/32999/diff/ Testing --- N/A Thanks, Ben Mahler

Re: Review Request 33465: Removed 'uuid' field from UPDATE call.

2015-04-24 Thread Ben Mahler
'uuid' and add a TODO on the existing hacks here an in sched.cpp? Would love to see the hacks removed in a future version! Also, looks like we need a new test for this case, yes? Mind sending a separate review for that before committing this one? - Ben Mahler On April 23, 2015, 5:48 a.m

Re: Review Request 33558: Add C++11 lambdas to the C++ style guide.

2015-04-27 Thread Ben Mahler
On April 28, 2015, 2:08 a.m., Ben Mahler wrote: docs/mesos-c++-style-guide.md, line 162 https://reviews.apache.org/r/33558/diff/3/?file=942044#file942044line162 When would we use capture by reference? Seems prone to mistakes? Michael Park wrote: When would we use capture

Re: Review Request 32838: Use unrestricted union to remove dynamic allocation from Option.

2015-04-30 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32838/#review82200 --- Ship it! Ship It! - Ben Mahler On April 30, 2015, 4:22 p.m

Re: Review Request 32837: Re-order structs in Slave State.hpp to prevent forward declaration dependency.

2015-04-30 Thread Ben Mahler
/#comment132903 It seems a bit odd to put the top-level state in the middle, before one can see the tree structure through the order they're listed in the header. Why not just forward declare above? - Ben Mahler On April 30, 2015, 4:22 p.m., Joris Van Remoortere wrote

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

2015-04-29 Thread Ben Mahler
a `followSymlinks` boolean? Just thinking of how to provide a complete and consistent interface that is simple to reason about. - Ben Mahler On March 11, 2015, 5:06 p.m., Bernd Mathiske wrote: --- This is an automatically generated e

Re: Review Request 33448: Added Eclipse-specific files to the gitignore list

2015-04-29 Thread Ben Mahler
/thread/7mu6lzkv3b6hngww - Ben Mahler On April 22, 2015, 8:36 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33448

Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.

2015-04-29 Thread Ben Mahler
On April 29, 2015, 8:24 p.m., Joris Van Remoortere wrote: I was surprised to see new functionality in this patch since the summary was code movement :) Just a quick note on DRY below. On April 29, 2015, 8:24 p.m., Joris Van Remoortere wrote: src/master/framework.cpp, line 187

Re: Review Request 33828: Fix for MESOS-2690. Issue with enable-optimize build.

2015-05-04 Thread Ben Mahler
, Ben Mahler and Cody Maloney. Bugs: MESOS-2690 https://issues.apache.org/jira/browse/MESOS-2690 Repository: mesos Description --- See Summary. Diffs - configure.ac 589ae97d0432370b462576cd1985544564893999 Diff: https://reviews.apache.org/r/33828/diff

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

2015-05-07 Thread Ben Mahler
--- On May 7, 2015, 5:47 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33935

Re: Review Request 33490: MESOS-2633 Avoid adding duplicate tasks

2015-05-07 Thread Ben Mahler
out? Could we get some more context on this change? What motivated it? Is there a bug? When can it occur? What happens if it does? - Ben Mahler On May 7, 2015, 8:26 a.m., Marco Massenzio wrote: --- This is an automatically generated e

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

2015-05-07 Thread Ben Mahler
some food for thought. :) - Ben Mahler On May 6, 2015, 10:35 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33918

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

2015-05-07 Thread Ben Mahler
. ``` --oversubscription=disable -- this for disabling --oversubscription=conservative --oversubscription=bayesian --oversubscription=custom ``` Did you have something in mind for exposing this? - Ben Mahler On May 6, 2015, 10:35 p.m., Jie Yu wrote

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

2015-05-07 Thread Ben Mahler
-redhat-linux-gnu) libcurl/7.15.5 OpenSSL/0.9.8b zlib/1.2.3 libidn/0.6.5' I0507 05:42:34.905570 31000 http.cpp:237] HTTP GET for /slave(1)/state.json from 127.0.0.1:5051 with User-Agent='curl/7.15.5 (x86_64-redhat-linux-gnu) libcurl/7.15.5 OpenSSL/0.9.8b zlib/1.2.3 libidn/0.6.5' Thanks, Ben Mahler

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

2015-05-07 Thread Ben Mahler
) libcurl/7.15.5 OpenSSL/0.9.8b zlib/1.2.3 libidn/0.6.5' I0507 05:42:34.905570 31000 http.cpp:237] HTTP GET for /slave(1)/state.json from 127.0.0.1:5051 with User-Agent='curl/7.15.5 (x86_64-redhat-linux-gnu) libcurl/7.15.5 OpenSSL/0.9.8b zlib/1.2.3 libidn/0.6.5' Thanks, Ben Mahler

Re: Review Request 33935: Improved HTTP request logging for master/slave endpoints.

2015-05-07 Thread Ben Mahler
`this` is in general dangeous because you want to make sure the callback is invoked before this object is destructured. It's not obviously here that this is the case. Ben Mahler wrote: We need `http` which is a member variable. The last line is essentialy `return this-http.observe

Re: Review Request 33505: Add state-summary endpoint to master.

2015-05-05 Thread Ben Mahler
of the motivation for the new endpoint and its format. A bit tough to tell what a sample response looks like without reading through this diff carefully. :) - Ben Mahler On May 5, 2015, 9:51 p.m., Joris Van Remoortere wrote

Re: Review Request 33792: Add InsensitiveHashMap.

2015-05-04 Thread Ben Mahler
.) Review request for mesos and Ben Mahler. Bugs: MESOS-328 https://issues.apache.org/jira/browse/MESOS-328 Repository: mesos Description --- Add InsensitiveHashMap. Diffs - 3rdparty/libprocess/3rdparty/stout/include/Makefile.am

Re: Review Request 33558: Add C++11 lambdas to the C++ style guide.

2015-05-04 Thread Ben Mahler
On April 28, 2015, 2:08 a.m., Ben Mahler wrote: docs/mesos-c++-style-guide.md, lines 206-208 https://reviews.apache.org/r/33558/diff/3/?file=942044#file942044line206 What does it mean to pass a lambda to `socket.send`? Doesn't look like this is functionality provided

Re: Review Request 33828: Fix for MESOS-2690. Issue with enable-optimize build.

2015-05-04 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33828/#review82473 --- Ship it! Thanks for the quick turnaround! - Ben Mahler On May 4

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

2015-05-11 Thread Ben Mahler
, 1:46 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33153/ --- (Updated April 14, 2015, 1:46 a.m

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

2015-05-11 Thread Ben Mahler
/#comment134222 unique_ptr is not a POD, so this will still try to run the destructor of the function during exit of the program. Can you just use a raw pointer here and leak it? (Not ideal but we use this in many places) - Ben Mahler On May 9, 2015, 5:07 p.m., haosdent huang wrote

Re: Review Request 33792: Extend hashmap to support custom equality and hash

2015-05-11 Thread Ben Mahler
of this file - Ben Mahler On May 9, 2015, 3:36 p.m., haosdent huang wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33792

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

2015-05-11 Thread Ben Mahler
973f0517400786fb16f86914d1d077c88965c9da Diff: https://reviews.apache.org/r/33154/diff/ Testing --- make check Thanks, Ben Mahler

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

2015-05-11 Thread Ben Mahler
e-mail. To reply, visit: https://reviews.apache.org/r/33154/#review80444 --- On April 14, 2015, 1:46 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply

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

2015-05-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33155/#review80397 --- On April 14, 2015, 1:46 a.m., Ben Mahler wrote

Review Request 33934: Added the client address to http::Request.

2015-05-06 Thread Ben Mahler
to the existing tests to ensure it is being set. Thanks, Ben Mahler

Review Request 33933: Slowed the webui polling for larger clusters.

2015-05-06 Thread Ben Mahler
7ee3d3d386857cf9dc47bcdd694308d980211dff Diff: https://reviews.apache.org/r/33933/diff/ Testing --- Ran the webui manually. Thanks, Ben Mahler

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

2015-05-12 Thread Ben Mahler
On May 11, 2015, 10:19 p.m., Ben Mahler wrote: src/slave/slave.cpp, line 155 https://reviews.apache.org/r/34016/diff/1/?file=954524#file954524line155 unique_ptr is not a POD, so this will still try to run the destructor of the function during exit of the program. Can

Re: Review Request 31667: Piped hashmapSlaveID, Resources from allocator through to sorter.

2015-05-13 Thread Ben Mahler
? src/tests/sorter_tests.cpp https://reviews.apache.org/r/31667/#comment134701 Ditto here, let's flip the arguments. - Ben Mahler On May 10, 2015, 12:21 a.m., Michael Park wrote: --- This is an automatically generated e-mail

Re: Review Request 33376: MESOS-2633 Moved struct Framework methods to their own implementation class.

2015-05-13 Thread Ben Mahler
'`Framework`'. - Ben Mahler On May 13, 2015, 8:12 p.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33376

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

2015-05-12 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33154/#review83463 --- On May 12, 2015, 2:51 a.m., Ben Mahler wrote: --- This is an automatically

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

2015-05-12 Thread Ben Mahler
Diff: https://reviews.apache.org/r/33154/diff/ Testing --- make check Thanks, Ben Mahler

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

2015-05-12 Thread Ben Mahler
/tests/master_tests.cpp 75ffadae64ece4e3ff53abeefa5f6e8e3690d402 src/tests/partition_tests.cpp 1018e479a77a6b533f2dd392fedbdccb80e3ac1f src/tests/slave_tests.cpp 04e79ec061e767a82d40242be7884967942c50b6 Diff: https://reviews.apache.org/r/33155/diff/ Testing --- make check Thanks, Ben

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

2015-05-12 Thread Ben Mahler
check Thanks, Ben Mahler

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

2015-05-12 Thread Ben Mahler
://reviews.apache.org/r/33153/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 34375: Removed use of namespace aliases.

2015-05-18 Thread Ben Mahler
considered, because there is no way to be able to write just `http::Response` otherwise, is there? Seems quite verbose to write process::http everywhere, and on the otherhand just having `Request` or `Response` seems to miss the context of it being http, thoughts? - Ben Mahler On May 18, 2015, 11

Review Request 34387: Moved up Slave and Framework structs in master.hpp.

2015-05-18 Thread Ben Mahler
- src/master/master.hpp da0a83510784f4f7dbd933e666ac12c04c413a62 Diff: https://reviews.apache.org/r/34387/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 34389: Removed Master::getSlave.

2015-05-18 Thread Ben Mahler
da0a83510784f4f7dbd933e666ac12c04c413a62 src/master/master.cpp eaea79df2c693d15087d70b3c9b988e57c894f8e src/master/validation.cpp 20a6ac833ec5dc437f80159a9234c7b94d86ba29 Diff: https://reviews.apache.org/r/34389/diff/ Testing --- make check Thanks, Ben Mahler

Re: Review Request 34294: Add an updated committers document to replace the stale svn page.

2015-05-19 Thread Ben Mahler
/ Testing --- See markdown in: https://reviews.apache.org/r/34295/ Thanks, Ben Mahler

Re: Review Request 34295: Added maintainers documentation.

2015-05-19 Thread Ben Mahler
in the latter). - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34295/#review84007 --- On May 15, 2015, 10:25 p.m., Ben Mahler

Re: Review Request 34420: Updated the committer doc regarding checking JIRA before committing.

2015-05-19 Thread Ben Mahler
(including the commit message in a comment), if applicable? - Ben Mahler On May 19, 2015, 5:40 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34420

Re: Review Request 34420: Updated the committer doc regarding checking JIRA before committing.

2015-05-19 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34420/#review84404 --- Ship it! Otherwise looks good! - Ben Mahler On May 19, 2015, 5

Re: Review Request 34438: Removed unnecessary freeaddrinfo in hostname if getaddrinfo returns error.

2015-05-19 Thread Ben Mahler
for you. 3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp https://reviews.apache.org/r/34438/#comment135629 Let's move the * here to be in the right place. - Ben Mahler On May 19, 2015, 9:44 p.m., Chi Zhang wrote

Re: Review Request 33793: HTTP headers should be considered case-insensitive.

2015-05-19 Thread Ben Mahler
, so flip the order of the arguments here. Also, these should fit on one line, ditto below. 3rdparty/libprocess/src/tests/http_tests.cpp https://reviews.apache.org/r/33793/#comment135597 Ditto here. Don't bother with stringify. - Ben Mahler On May 9, 2015, 3:37 p.m., haosdent huang

Re: Review Request 34068: The test case of extend hashmap to support custom equality and hash

2015-05-19 Thread Ben Mahler
, it's ok to continue checking things. Can you re-organize this a bit, it's hard to follow that you're testing the things we care about. Also, how about just sticking with 'put' and 'get', rather than also using '[]' and 'contains' here? - Ben Mahler On May 12, 2015, 12:56 a.m

Re: Review Request 33792: Extend hashmap to support custom equality and hash

2015-05-19 Thread Ben Mahler
/libprocess/3rdparty/stout/include/stout/hashmap.hpp https://reviews.apache.org/r/33792/#comment135580 Can we change this to 'Equal'? s/Pred/Equal/ This describes what it does, and I also noticed you called it 'Equal' in https://reviews.apache.org/r/33793 ;) - Ben Mahler On May 12

Re: Review Request 34970: Cleaned up and generalized NoExecutorScheduler to be more configurable.

2015-06-04 Thread Ben Mahler
/diff/ Testing (updated) --- The existing no executor framework test picks this up. Since we do not have an non-zero estimator yet, I modified the slave to send revocable resources in order to manually test the revocable case. Thanks, Ben Mahler

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

2015-06-05 Thread Ben Mahler
://reviews.apache.org/r/33271/#comment138833 Do we really want to encourage taking a const of a POD type? In general we have not been doing this, so it seems pretty inconsistent to put it in this example. Also, looks like we need a space before the openening parenthesis. - Ben Mahler On June 2

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-03 Thread Ben Mahler
of these. For (1) we could just add the shutdown message expectation to `PartitionTest.PartitionedSlave`. For (2), we have SlaveTest.PingTimeoutNoPings, SlaveTest.PingTimeoutNoPings. - Ben Mahler On May 28, 2015, 11:13 p.m., Adam B wrote

Re: Review Request 33296: Added a flag which controls libprocess firewall initialzation.

2015-06-08 Thread Ben Mahler
, rules.disabled_endpoints().paths()) { paths.insert(path); } ``` src/slave/main.cpp https://reviews.apache.org/r/33296/#comment139396 Don't you need a utility include for move? - Ben Mahler On June 8, 2015, 12:11 p.m., Alexander Rojas wrote

Re: Review Request 33295: Added firewall mechanism to control access on libprocess http endpoints.

2015-06-08 Thread Ben Mahler
/process_tests.cpp https://reviews.apache.org/r/33295/#comment139429 Is it possible to just pass `{}` here for empty vector? - Ben Mahler On June 8, 2015, 10:09 a.m., Alexander Rojas wrote: --- This is an automatically generated e-mail

Re: Review Request 35118: Made updateSlave() update its 'totalResources'.

2015-06-08 Thread Ben Mahler
On June 5, 2015, 9:58 p.m., Vinod Kone wrote: src/master/master.cpp, line 3462 https://reviews.apache.org/r/35118/diff/1/?file=980131#file980131line3462 woah. didn't realize this was handled automagically by the install handler. Yeah, we didn't do this for framework provided

Re: Review Request 35120: Use non-POD type for alias example in c++ style guide.

2015-06-08 Thread Ben Mahler
/#comment139436 By the way, I removed the std:: prefix here. docs/mesos-c++-style-guide.md https://reviews.apache.org/r/35120/#comment139437 Also I added const to the 'values', was that not the intent? - Ben Mahler On June 5, 2015, 9:34 a.m., Joris Van Remoortere wrote

Re: Review Request 34921: Code Refactor: float the bytes to get rid of the truncate fraction part in function datasize.

2015-06-08 Thread Ben Mahler
Do you still need the fraction on 1024.0 with this? Maybe a note is needed: ``` // Ensure bytes is treated as floating point for the math below. bytes = float(bytes) ... ``` - Ben Mahler On June 2, 2015, 3:28 a.m., weitao zhou wrote

Re: Review Request 35120: Use non-POD type for alias example in c++ style guide.

2015-06-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35120/#review87111 --- Ship it! Ship It! - Ben Mahler On June 5, 2015, 9:34 a.m

Re: Review Request 32999: Added a document for engineering principles and practices.

2015-06-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32999/#review86224 --- On April 23, 2015, 11:15 p.m., Ben Mahler wrote

Re: Review Request 34970: Cleaned up and generalized NoExecutorScheduler to be more configurable.

2015-06-05 Thread Ben Mahler
estimator yet, I modified the slave to send revocable resources in order to manually test the revocable case. Thanks, Ben Mahler

Re: Review Request 34970: Cleaned up and generalized NoExecutorScheduler to be more configurable.

2015-06-05 Thread Ben Mahler
. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34970/#review86767 --- On June 5, 2015, 5:52 a.m., Ben Mahler wrote

Re: Review Request 30774: Fetcher Cache

2015-06-05 Thread Ben Mahler
I can't tell why slave id is being passed here, is there something subtle going on? - Ben Mahler On May 21, 2015, 4:05 p.m., Bernd Mathiske wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 34970: Cleaned up and generalized NoExecutorScheduler to be more configurable.

2015-06-05 Thread Ben Mahler
--- On June 5, 2015, 7:56 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34970/ --- (Updated

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

2015-06-02 Thread Ben Mahler
to be able to declare when an argument is required to avoid these manual checks entirely. :) - Ben Mahler On May 29, 2015, 1:10 a.m., Marco Massenzio wrote: --- This is an automatically generated e-mail. To reply, visit: https

Re: Review Request 34195: Refactoring to use FlagsBase common functionality

2015-06-02 Thread Ben Mahler
On June 2, 2015, 5:52 p.m., Ben Mahler wrote: I'm seeing the following pattern: ``` if (flags.master.isNone()) { cerr flags.usage(Missing required option --master) endl; return EXIT_FAILURE; } ``` Why not leverage EXIT here? ``` if (flags.master.isNone

Re: Review Request 34306: Added capabilities field to FrameworkInfo.

2015-06-02 Thread Ben Mahler
with the master? Would help debugging, as I was just tripped up by forgetting to set it. Also, might be easy to fold in to printing the 'checkpoint' capability (maybe also at some point folding it in to the Capabilities enum). - Ben Mahler On May 21, 2015, 12:42 a.m., Vinod Kone wrote

Re: Review Request 34729: Updated slave to send total amount of oversubscribed resources.

2015-06-02 Thread Ben Mahler
/#comment138292 Is the lack of a delay() in this case a bug? Doesn't this break the `forwardOversubscribedResources` loop? Is there something else going on that ensures the loop continues..? - Ben Mahler On May 29, 2015, 12:30 a.m., Vinod Kone wrote

Re: Review Request 34956: Refactored the queueing discipline data structure.

2015-06-02 Thread Ben Mahler
inconsistent? - Ben Mahler On June 2, 2015, 10:04 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34956

Review Request 34968: Fixed a deadlock in libprocess Metrics initialization.

2015-06-02 Thread Ben Mahler
0x0047f06a in main () ``` Thanks, Ben Mahler

Review Request 34969: Print executor status consistently in the example frameworks.

2015-06-02 Thread Ben Mahler
src/examples/persistent_volume_framework.cpp 6a0c0cb61a06d1a0e7608fe2447167c18111ea43 Diff: https://reviews.apache.org/r/34969/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 35262: Logged framework capabilities during (re-)registration.

2015-06-09 Thread Ben Mahler
with checkpointing disabled and capabilities [ REVOCABLE_RESOURCES ] ``` Thanks, Ben Mahler

Re: Review Request 35239: Update the JSON model for Resources to display their revocablility attribute.

2015-06-09 Thread Ben Mahler
difficult to make further changes without breaking people. For example, if we add 'total_resources' as done here, but then we want to show the role, how do we do that without breaking people's code that exactly matches cpus ? - Ben Mahler On June 9, 2015, 12:38 a.m., Jiang Yan Xu wrote

Re: Review Request 34911: Added a test for launching an executor on revocable resources.

2015-06-03 Thread Ben Mahler
`'? - Ben Mahler On June 1, 2015, 11:15 p.m., Vinod Kone wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34911/ --- (Updated June

Re: Review Request 35118: Made updateSlave() update its 'totalResources'.

2015-06-09 Thread Ben Mahler
On June 9, 2015, 5:36 p.m., Niklas Nielsen wrote: src/master/master.cpp, lines 3513-3514 https://reviews.apache.org/r/35118/diff/1/?file=980131#file980131line3513 Does it make sense to point to some documentation (if it exists already, or inline) about how this resource math will

Re: Review Request 34921: Code Refactor: float the bytes to get rid of the truncate fraction part in function datasize.

2015-06-09 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34921/#review87336 --- Ship it! Ship It! - Ben Mahler On June 9, 2015, 2:58 a.m

Re: Review Request 35697: mesos: Fixed test names.

2015-06-22 Thread Ben Mahler
are inconcistent on the Test suffix, e.g. MasterTest vs CRAMMD5Authentication. See two options: (1) We can use Test to signify that there is a fixture, if that's helpful. So remove any Test suffixes from non-fixture tests. (2) We can use Test as a suffix for all tests. Thoughts? - Ben Mahler On June 20

Re: Review Request 35696: libprocess: Fixed test names.

2015-06-22 Thread Ben Mahler
is that some tests are Process and some tests are named JsonTest. The latter tends to be forced when we have a test fixture (e.g. MasterTest). - Ben Mahler On June 20, 2015, 7:14 p.m., Michael Park wrote: --- This is an automatically

Re: Review Request 35714: Added a new HTTP response type: PreconditionFailed.

2015-06-22 Thread Ben Mahler
not contain explicit pre-conditions, it seems a little non-idiomatic to return 412..? - Ben Mahler On June 22, 2015, 5:08 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r

Re: Review Request 35663: Added a helper in Resources to get all scalar resources.

2015-06-19 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35663/#review88588 --- Ship it! Ship It! - Ben Mahler On June 19, 2015, 7:57 p.m., Jie

Re: Review Request 35433: Sent StatusUpdates if checkpointed resources don't exist on the slave.

2015-06-19 Thread Ben Mahler
On June 14, 2015, 10:46 a.m., Benjamin Hindman wrote: Just so I understand, does this mean if we happen to get in the unfortunate situation where a slave has neglected to get the dynamic reservation because it was just starting up and then it gets the task launch it will shutdown the

Re: Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

2015-06-19 Thread Ben Mahler
/sorter_tests.cpp (lines 396 - 398) https://reviews.apache.org/r/35664/#comment141183 Hm.. wonder if we can do the following now in these tests? ``` EXPECT_EQ({a, b}, sorter.sort()); ``` - Ben Mahler On June 19, 2015, 10:21 p.m., Jie Yu wrote

Re: Review Request 35671: Fixed a bug in test filter that prevent some tests from being launched.

2015-06-19 Thread Ben Mahler
://reviews.apache.org/r/35671/#comment141180 Flip the order? - Ben Mahler On June 19, 2015, 10:22 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35671

Re: Review Request 35664: Improved the performance of DRF sorter by caching the scalars.

2015-06-19 Thread Ben Mahler
(); } if (total 0.0) { ... } } ``` Ditto for computing `allocation` below. Seems a bit non-intuitive that the call to `get` is aggregating across `Resource` objects. One would guess that it is just pulling out the scalar by that name. - Ben Mahler On June 19, 2015, 9:09

Re: Review Request 35815: Testing JSON paylodad in ZooKeeper

2015-06-25 Thread Ben Mahler
On June 25, 2015, 12:38 a.m., Vinod Kone wrote: src/tests/master_contender_detector_tests.cpp, line 877 https://reviews.apache.org/r/35815/diff/3/?file=991386#file991386line877 s/EXPECT/ASSERT/ as this is the last expectation in the test. Marco Massenzio wrote:

Re: Review Request 35849: Added a precision test for Resources.

2015-06-24 Thread Ben Mahler
://reviews.apache.org/r/35849/#comment141852 Mind leaving a note for posterity as to why it's disabled? E.g. ``` // NOTE: This is disabled due to MESOS-1187. ``` - Ben Mahler On June 24, 2015, 10:10 p.m., Jie Yu wrote

Re: Review Request 35849: Added a precision test for Resources.

2015-06-24 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35849/#review89261 --- Ship it! Ship It! - Ben Mahler On June 24, 2015, 10:10 p.m

Re: Review Request 34943: Added validation to flags.

2015-06-24 Thread Ben Mahler
(); }); ``` Thoughts? Thoughts? - Ben Mahler On June 18, 2015, 3:34 p.m., Benjamin Hindman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34943

Re: Review Request 35837: Fixed right angle brackets in hierarchical allocator.

2015-06-24 Thread Ben Mahler
/master/allocator/mesos/allocator.hpp include/mesos/master/allocator.hpp - Ben Mahler On June 24, 2015, 6:16 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35837

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-24 Thread Ben Mahler
) https://reviews.apache.org/r/29507/#comment141866 Revert this change? Looks odd that TERMINATING has the same two lines but doesn't have the newline. - Ben Mahler On June 22, 2015, 10:03 a.m., Adam B wrote

Re: Review Request 35816: Fixed the incorrect CHECK_EQ in updateAllocation.

2015-06-24 Thread Ben Mahler
we might want to express over-allocation, see: [MESOS-2930](https://issues.apache.org/jira/browse/MESOS-2930). - Ben Mahler On June 24, 2015, 3:35 p.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit

Re: Review Request 29507: Added Configurable Slave Ping Timeouts

2015-06-24 Thread Ben Mahler
this interact with the zookeeper session timeout? - Ben Mahler On June 22, 2015, 10:03 a.m., Adam B wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29507

Re: Review Request 35838: Added a TODO about RefusedFilter in hierarchical allocator.

2015-06-24 Thread Ben Mahler
if there is more of both revocable and non-revocable resources. - Ben Mahler On June 24, 2015, 6:17 p.m., Jie Yu wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35838

  1   2   3   4   5   6   7   8   9   >