Re: Review Request 47209: Establish TCP connection after backing off.

2016-05-12 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47209/#review133025 --- Ship it! Ship It! - Ben Mahler On May 13, 2016, 3:37 a.m

Re: Review Request 47209: Establish TCP connection after backing off.

2016-05-12 Thread Ben Mahler
e following? ``` // Ensure there is a link to the master before we start communicating with it. ``` - Ben Mahler On May 12, 2016, 6:18 p.m., David Robinson wrote: > > --- > This is an automatically generated e-mail. To r

Re: Review Request 46670: Added deprecated alias for `--authenticate_frameworks` master flag.

2016-05-12 Thread Ben Mahler
) <https://reviews.apache.org/r/46670/#comment197314> Mind committing this bit separately? You also need duration.hpp, error.hpp, none.hpp, stringify.hpp. - Ben Mahler On May 12, 2016, 10:38 p.m., Vinod Kone wrote: > > ---

Re: Review Request 46815: Updated mesos to work with new `flags.load()` signature.

2016-05-12 Thread Ben Mahler
tps://reviews.apache.org/r/46815/#comment197313> `->` here and everywhere else - Ben Mahler On May 12, 2016, 10:43 p.m., Vinod Kone wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https:/

Re: Review Request 46818: Updated libprocess tests to work with the new `flag.load()` signature.

2016-05-12 Thread Ben Mahler
(line 613) <https://reviews.apache.org/r/46818/#comment197312> Can you add an expectation that there are no warnings? - Ben Mahler On May 12, 2016, 10:42 p.m., Vinod Kone wrote: > > --- > This is an automatically gener

Re: Review Request 46863: Refactored FlagsBase::load() to move duplicate checking logic.

2016-05-12 Thread Ben Mahler
gs/flags.hpp (lines 883 - 884) <https://reviews.apache.org/r/46863/#comment197311> Shouldn't this print the `name` being looped over here so that we can see the two clashing names? - Ben Mahler On May 12, 2016, 10:42 p.m., Vinod Kone wrote: > > ---

Re: Review Request 46669: Added deprecation support to Flag name.

2016-05-12 Thread Ben Mahler
te all of these existing tests to expect no warnings? 3rdparty/stout/tests/flags_tests.cpp (line 502) <https://reviews.apache.org/r/46669/#comment197298> Can you use the `->` operator here? EXPECT? Also can you add an expectation on the message? - Ben Mahler On May

Re: Review Request 47209: Establish TCP connection after backing off.

2016-05-12 Thread Ben Mahler
-- > > (Updated May 11, 2016, 12:58 a.m.) > > > Review request for mesos, Ben Mahler, Ian Downes, and Cong Wang. > > > Bugs: MESOS-5330 > https://issues.apache.org/jira/browse/MESOS-5330 > > > Repository: mesos > > > Descrip

Re: Review Request 47028: Fixed a confusing log line in the allocator.

2016-05-11 Thread Ben Mahler
1488) <https://reviews.apache.org/r/47028/#comment197044> How about "No allocations performed"? It is a bit odd to refer to offers here since we don't know about offers in the allocator currently. - Ben Mahler On May 11, 2016, 5:58 p.m

Review Request 47192: Fixed a head-of-line blocking bug in libevent SSL socket.

2016-05-10 Thread Ben Mahler
... Connected to localhost. Escape character is '^]'. ``` Hit the endpoints: ``` $ curl --insecure https://localhost:5050/health ``` These curl requests hangs without this fix. Thanks, Ben Mahler

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

2016-05-09 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46325/#review132324 --- Ship it! Modulo comments. - Ben Mahler On May 7, 2016, 1:03

Re: Review Request 46491: Ensured subsequent kills are ignored after the task is reaped.

2016-05-09 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46491/#review132323 --- Ship it! Modulo comments. - Ben Mahler On May 7, 2016, 1:02

Re: Review Request 47091: Made http::internal::ConnectionProcess a managed Process.

2016-05-07 Thread Ben Mahler
- 3rdparty/libprocess/src/http.cpp 48f91d01555e760b1c4fd2cde684168d65f75e57 Diff: https://reviews.apache.org/r/47091/diff/ Testing --- Removed the hack put in place within the http::get path to validate the fix. Thanks, Ben Mahler

Re: Review Request 47091: Made http::internal::ConnectionProcess a managed Process.

2016-05-07 Thread Ben Mahler
context. Diffs (updated) - 3rdparty/libprocess/src/http.cpp 48f91d01555e760b1c4fd2cde684168d65f75e57 Diff: https://reviews.apache.org/r/47091/diff/ Testing --- Removed the hack put in place within the http::get path to validate the fix. Thanks, Ben Mahler

Re: Review Request 47091: Made http::internal::ConnectionProcess a managed Process.

2016-05-07 Thread Ben Mahler
- 3rdparty/libprocess/src/http.cpp 48f91d01555e760b1c4fd2cde684168d65f75e57 Diff: https://reviews.apache.org/r/47091/diff/ Testing --- Removed the hack put in place within the http::get path to validate the fix. Thanks, Ben Mahler

Review Request 47092: Removed misleading return values from process::network::.

2016-05-07 Thread Ben Mahler
9cc4acd11dba561f40c33bc9dabb35a452a80e62 3rdparty/libprocess/src/poll_socket.cpp e68c7836b6a79253fa646c1919d0f331f21ec131 3rdparty/libprocess/src/socket.cpp ec0e913aca30f92d4a0543ad1e685b897617bca3 Diff: https://reviews.apache.org/r/47092/diff/ Testing --- make check Thanks, Ben Mahler

Review Request 47091: Made http::internal::ConnectionProcess a managed Process.

2016-05-07 Thread Ben Mahler
: https://reviews.apache.org/r/47091/diff/ Testing --- Removed the hack put in place within the http::get path to validate the fix. Thanks, Ben Mahler

Review Request 47090: Removed side-effects from MetricsTest.SnapshotAuthenticationEnabled.

2016-05-07 Thread Ben Mahler
1cda7b4ec31fcd06161925ce5788741f299217c7 Diff: https://reviews.apache.org/r/47090/diff/ Testing --- Ran in repetition. Thanks, Ben Mahler

Re: Review Request 46730: Cleanup syscalls logic.

2016-05-06 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46730/#review132098 --- Ship it! Ship It! - Ben Mahler On April 29, 2016, 1:03 p.m

Re: Review Request 46960: Remove un-necessary copying of `slave->tasks` in master.

2016-05-05 Thread Ben Mahler
rs[frameworkId].keys()) { ``` - Ben Mahler On May 4, 2016, 5:14 a.m., Anand Mazumdar wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > ht

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

2016-05-05 Thread Ben Mahler
wondering why there are two variables for these (that has a lot of context of the road you went down trying to consolidate them into just the timer variable). Likely they're more interested in what these represent. If we cha

Re: Review Request 46491: Ensured escalated() is not called after reaped() in command executor.

2016-05-05 Thread Ben Mahler
om above. src/launcher/http_command_executor.cpp (lines 712 - 717) <https://reviews.apache.org/r/46491/#comment195926> Ditto from above. - Ben Mahler On May 5, 2016, 3:37 p.m., Alexander Rukletsov wrote: > > -

Re: Review Request 46321: Renamed a variable in command executors for clarity.

2016-05-05 Thread Ben Mahler
uot;terminated" to the patch that reads the value? It's only written to in this patch AFAICT. - Ben Mahler On May 5, 2016, 3:37 p.m., Alexander Rukletsov wrote: > > --- &g

Re: Review Request 46165: Removed dependency on Boost.Foreach.

2016-05-03 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46165/#review131593 --- Ship it! Ship It! - Ben Mahler On May 3, 2016, 8:53 p.m

Re: Review Request 46945: Renamed a shadowing variable.

2016-05-03 Thread Ben Mahler
/include/stout/json.hpp (lines 749 - 751) <https://reviews.apache.org/r/46945/#comment195596> Could we avoid the 'elem' name in favor of 'element' or 'v' (full word or single letter)? Per our conversation, do we want to adjust the foreachpair above?

Re: Review Request 46804: Patched glog to eliminate system-level gflag library detection.

2016-04-28 Thread Ben Mahler
gging_unittest-logging_unittest.o] Error 1 ``` Thanks, Ben Mahler

Review Request 46805: Added backport of MESOS-5018 to 0.27.3.

2016-04-28 Thread Ben Mahler
--- This reflects the backport of MESOS-5018 to the 0.27.x branch. Diffs - CHANGELOG 0c5987779e5f0e2cb416dfe885f7ba836e3958eb Diff: https://reviews.apache.org/r/46805/diff/ Testing --- N/A Thanks, Ben Mahler

Review Request 46804: Patched glog to eliminate system-level gflag library detection.

2016-04-28 Thread Ben Mahler
ST(UserDefinedClass, logging) { ^ make[7]: *** [logging_unittest-logging_unittest.o] Error 1 ``` Thanks, Ben Mahler

Review Request 46753: Added backport of MESOS-5021 to 0.27.3.

2016-04-27 Thread Ben Mahler
--- This reflects the backport of MESOS-5021 to the 0.27.x branch. Diffs - CHANGELOG 937716068c126fd76b70e08f7654a659ba1029d8 Diff: https://reviews.apache.org/r/46753/diff/ Testing --- N/A Thanks, Ben Mahler

Re: Review Request 44255: Add metrics for {RESERVE, UNRESERVE, CREATE, DESTROY} offer operation.

2016-04-27 Thread Ben Mahler
master/scheduler_calls/type/kill: 512 ... ``` Also, this lets us distinguish between schedulers making calls and operators hitting the endpoints (the current patch treats these the same). We should get some feedback from Vinod Kone and Anand Mazumdar since they've been working on the HTTP API.

Re: Review Request 46730: Cleanup syscalls logic.

2016-04-27 Thread Ben Mahler
background information in the review description? - Ben Mahler On April 27, 2016, 11:44 a.m., Tomasz Janiszewski wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 46610: Fix 'pivot_root is not available' error on ARM.

2016-04-26 Thread Ben Mahler
PowerPC) I've gone ahead and committed it. The cleanup mentioned by haosdent sounds great if it removes unnecessary #ifdef logic. Let me know when the cleanup is available! - Ben Mahler On April 24, 2016, 11:36 a.m., Tomasz Janiszewski

Review Request 46663: Added 0.28.2 to the CHANGELOG.

2016-04-25 Thread Ben Mahler
--- This reflects the backport of MESOS-4705 to the 0.28.x branch. Diffs - CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 Diff: https://reviews.apache.org/r/46663/diff/ Testing --- N/A Thanks, Ben Mahler

Review Request 46664: Added 0.27.3 to the CHANGELOG.

2016-04-25 Thread Ben Mahler
--- This reflects the backport of MESOS-4705 to the 0.27.x branch. Diffs - CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 Diff: https://reviews.apache.org/r/46664/diff/ Testing --- N/A Thanks, Ben Mahler

Review Request 46665: Added 0.26.2 to the CHANGELOG.

2016-04-25 Thread Ben Mahler
--- This reflects the backport of MESOS-4705 to the 0.26.x branch. Diffs - CHANGELOG 57f5457640c9b7ca2cb7707f2962dd0421f090f7 Diff: https://reviews.apache.org/r/46665/diff/ Testing --- N/A Thanks, Ben Mahler

Re: Review Request 44379: Use tokens size to parse perf stat format.

2016-04-25 Thread Ben Mahler
nternal::normalize(tokens[2]), tokens[3]}); } +// Bail out if the format is not recognized. return Error("Unexpected number of fields"); } }; ``` - Ben Mahler On April 18, 2016, 5:41 a.m., fan du wrote: > > --- >

Re: Review Request 46325: Updated HTTP command executor to support kill policy in Kill event.

2016-04-21 Thread Ben Mahler
looks like a bug fix? Any reason it is being bundled with this patch? - Ben Mahler On April 21, 2016, 2:28 p.m., Alexander Rukletsov wrote: > > --- > This is an automa

Re: Review Request 46323: Propagated KillPolicy in kill task from scheduler to executor.

2016-04-21 Thread Ben Mahler
) <https://reviews.apache.org/r/46323/#comment193488> const & for both of these since the rhs is not a temporary - Ben Mahler On April 21, 2016, 2:28 p.m., Alexander Rukletsov wrote: > > --- > This is an automatical

Re: Review Request 46324: Corrected indentation in HttpCommandExecutor.

2016-04-18 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46324/#review129463 --- Ship it! Ship It! - Ben Mahler On April 18, 2016, 12:44 p.m

Re: Review Request 46323: Propagated KillPolicy in kill task from scheduler to executor.

2016-04-18 Thread Ben Mahler
in the handler to check the optionality. - Ben Mahler On April 18, 2016, 12:44 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To reply, visit:

Re: Review Request 46322: Added KillPolicy to scheduler and executor Kill protobuf messages.

2016-04-18 Thread Ben Mahler
overrides task_info.kill_policy. Perhaps for now, we just explicitly state that the grace period may be "overridden" (or "adjusted"?) in order to give more or less time to a graceful kill that is in progress. - Ben Mahler On April 18, 2016, 12:43 p.m., Alexander

Re: Review Request 45304: Change Call and Event Type enums in executor.proto optional.

2016-04-18 Thread Ben Mahler
new enum value. Ditto below. - Ben Mahler On March 25, 2016, 9:25 p.m., Yong Tang wrote: > > --- > This is an automatically generated e-mail. To reply, vis

Re: Review Request 45317: Change Call and Event Type enums in scheduler.proto optional.

2016-04-18 Thread Ben Mahler
tps://reviews.apache.org/r/45317/#comment192865> Yikes, this case isn't actually unreachable! Per MESOS-2664 and MESOS-3754, please avoid 'default' in favor of an explicit 'case UNKNOWN' so that the compiler helps us catch all switches when we introduce a new enum value. -

Re: Review Request 45342: Make the Action enum optional to support upgrades (MESOS-5031).

2016-04-18 Thread Ben Mahler
code from compiling when a new enum value is introduced but not handled. Make sense? Ditto for the recent executor.proto and scheduler.proto changes. - Ben Mahler On March 29, 2016, 2:04 a.m., Yong Tang wrote: > > --

Review Request 46339: Fixed the flaky MasterSlaveReconciliation.ReconcileRace test.

2016-04-18 Thread Ben Mahler
/master_slave_reconciliation_tests.cpp 71fb78afe6ddca061dd05cfda7bbf17d4b3ea834 Diff: https://reviews.apache.org/r/46339/diff/ Testing --- Ran in repetition. Thanks, Ben Mahler

Re: Review Request 46175: Added a test for slavePostFetchHook.

2016-04-13 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46175/#review128790 --- Ship it! Modulo comments from before. - Ben Mahler On April

Re: Review Request 46173: Added a slave post fetch hook.

2016-04-13 Thread Ben Mahler
tps://reviews.apache.org/r/46173/#comment192234> Would you mind getting the quotes on the same line? ``` LOG(WARNING) << "Failed to execute slave post fetch hook for module" << " '" << name << "': " &l

Re: Review Request 46175: Added a test for slavePostFetchHook.

2016-04-13 Thread Ben Mahler
43> Why `docker.get()->ps` instead of `docker->ps`? src/tests/hook_tests.cpp (line 843) <https://reviews.apache.org/r/46175/#comment192242> Why `docker.get()->rm` instead of `docker->rm`? - Ben Mahler On April 13, 2016, 10:33 p.m., Jie Yu wrote: > > ---

Re: Review Request 46027: Documented `libprocess` helper function.

2016-04-13 Thread Ben Mahler
> On April 13, 2016, 9:33 p.m., Ben Mahler wrote: > > 3rdparty/libprocess/src/process.cpp, line 559 > > <https://reviews.apache.org/r/46027/diff/2/?file=1340142#file1340142line559> > > > > s/. A/. A/ > > > > Thanks for clarifying, s

Re: Review Request 46027: Documented `libprocess` helper function.

2016-04-13 Thread Ben Mahler
) <https://reviews.apache.org/r/46027/#comment192211> s/. A/. A/ Thanks for clarifying, sorry for the confusion! - Ben Mahler On April 12, 2016, 12:23 a.m., Neil Conway wrote: > > --- > This is an automatically gener

Re: Review Request 46029: Mark a few private functions `static` in libprocess tests.

2016-04-13 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46029/#review128748 --- Ship it! Ship It! - Ben Mahler On April 12, 2016, 12:13 a.m

Re: Review Request 46121: Fixed an assumption of direct /tmp access in a slave test.

2016-04-12 Thread Ben Mahler
indeed using a temporary directory. - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46121/#review128578 ------- On

Re: Review Request 46121: Fixed an assumption of direct /tmp access in a slave test.

2016-04-12 Thread Ben Mahler
ated e-mail. To reply, visit: https://reviews.apache.org/r/46121/#review128580 --- On April 12, 2016, 11:15 p.m., Ben Mahler wrote: > > --- > This is an automatically generated

Review Request 46121: Fixed an assumption of direct /tmp access in a slave test.

2016-04-12 Thread Ben Mahler
due to /tmp/stdout being owned by root) Thanks, Ben Mahler

Re: Review Request 45932: Adds task information to container resource usage information.

2016-04-12 Thread Ben Mahler
.org/r/45932/#comment192010> How about key1,value1 and key2,value2? I'm not sure if there's any value in having task / executor within the names. - Ben Mahler On April 12, 2016, 5:03 a.m., Zhitao Li wrote: > > -

Re: Review Request 45572: Add labels to ExecutorInfo and deprecate source.

2016-04-12 Thread Ben Mahler
otobuf(createLabel("key1", "value1"))), labels_->values[0]); ``` src/tests/oversubscription_tests.cpp (lines 230 - 231) <https://reviews.apache.org/r/45572/#comment191949> My suggestion for earlier was just to avoid using words like "foo" and "bar

Re: Review Request 45941: Add check in agent for incorrect oversubscribed resource.

2016-04-12 Thread Ben Mahler
. src/slave/slave.cpp (line 4972) <https://reviews.apache.org/r/45941/#comment191911> newline here missing an 'i' in 'Oversubscrbable' src/slave/slave.cpp (line 4976) <https://reviews.apache.org/r/45941/#comment191912> s/.get()./->/ - Ben

Re: Review Request 46024: Avoided misleading locking in the libprocess SocketManager.

2016-04-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46024/#review128319 --- Ship it! Ship It! - Ben Mahler On April 11, 2016, 2:36 p.m

Re: Review Request 46053: Added logic to validate for non-fractional GPU requests in the master.

2016-04-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46053/#review128310 --- Ship it! Ship It! - Ben Mahler On April 11, 2016, 10:11 p.m

Re: Review Request 45969: Fixed indent in Nvidia GPU test.

2016-04-11 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45969/#review128309 --- Ship it! Ship It! - Ben Mahler On April 11, 2016, 10:19 p.m

Re: Review Request 45970: Added Nvidia GPU test to verify error when requesting fractional GPUs.

2016-04-11 Thread Ben Mahler
/nvidia_gpu_isolator_tests.cpp (lines 217 - 223) <https://reviews.apache.org/r/45970/#comment191734> How about no newlines between these since they are related? - Ben Mahler On April 11, 2016, 10:18 p.m., Kevin Klues wrote: > > --- > This is a

Re: Review Request 46029: Mark a few private functions `static` in libprocess tests.

2016-04-11 Thread Ben Mahler
/process_tests.cpp <https://reviews.apache.org/r/46029/#comment191701> Is this a complete header audit? Why only this file? I'd suggest we remove this part of the diff. - Ben Mahler On April 11, 2016, 9:01 p.m., Neil C

Re: Review Request 46028: Improved comments in SocketManager::next().

2016-04-11 Thread Ben Mahler
less of your changes) seems to be that the socket is an outbound socket? Is that true? - Ben Mahler On April 11, 2016, 2:35 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http

Re: Review Request 46024: Avoided misleading locking in the libprocess SocketManager.

2016-04-11 Thread Ben Mahler
eview but since they wrote the code I'd like them to be aware of this change. - Ben Mahler On April 11, 2016, 2:36 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://revi

Re: Review Request 46027: Documented `libprocess` helper function.

2016-04-11 Thread Ben Mahler
it is sent by a libprocess instance. - Ben Mahler On April 11, 2016, 2:35 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 46026: Documented `Socket::shutdown()` member function in libprocess.

2016-04-11 Thread Ben Mahler
was made implicit and no control was given to the caller. - Ben Mahler On April 11, 2016, 2:35 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: &

Re: Review Request 46025: Clarified comments on socket data structures in SocketManager.

2016-04-11 Thread Ben Mahler
/reviews.apache.org/r/46025/#comment191679> Why didn't you update this comment? Is the set of values within `temps` equivalent to `dispose`? Would be great to explain the distinction here. - Ben Mahler On April 11,

Re: Review Request 46030: Used initializer_list for collections in libprocess tests.

2016-04-11 Thread Ben Mahler
to an initializer list. - Ben Mahler On April 11, 2016, 9:01 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 45489: Replaced reinterpret_cast with static_cast in libprocess.

2016-04-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45489/#review127941 --- Ship it! Ship It! - Ben Mahler On March 30, 2016, 4:16 p.m

Re: Review Request 45590: Made `Delegate` and `Handlers` libprocess tests less fragile.

2016-04-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45590/#review127940 --- Ship it! Ship It! - Ben Mahler On April 1, 2016, 4:22 p.m

Re: Review Request 45932: Add stripped TaskInfo's to ResourceUsage.Executor message.

2016-04-08 Thread Ben Mahler
iews.apache.org/r/45932/#comment191318> How about s/task_label/key/ s/task_label_value/value/ ? Will it fit on one line then? We generally avoid "foo" and "bar" in favor of things like "key&q

Re: Review Request 45572: Add labels to ExecutorInfo and deprecate source.

2016-04-08 Thread Ben Mahler
include/mesos/mesos.proto (lines 496 - 498) <https://reviews.apache.org/r/45572/#comment191311> Any reason not to re-use the more descriptive comment on TaskInfo.labels? Specifically it would be nice to include the warning about the master keeping this data in memory. - Ben Mahl

Re: Review Request 45488: Removed an unnecessary `memset` from libprocess.

2016-04-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45488/#review127911 --- Ship it! Ship It! - Ben Mahler On March 30, 2016, 2:49 p.m

Re: Review Request 45941: Add check in agent for incorrect oversubscribed resource.

2016-04-08 Thread Ben Mahler
cator since that just happens to be where the failure manifests. Perhaps something like: ``` // Oversubscrbable resources must be considered revocable. // // TODO(bmahler): Consider tagging input as revocable // rather than rejecting and crashing here. ``` -

Re: Review Request 45572: Add labels to ExecutorInfo and deprecate source.

2016-04-08 Thread Ben Mahler
gt; > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45572/ > ----------- > > (Updated April 1, 2016, 1:42 a.m.) > > > Rev

Re: Review Request 45942: Updated the webui to include GPU metrics.

2016-04-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45942/#review127891 --- Ship it! Ship It! - Ben Mahler On April 8, 2016, 8:36 p.m

Re: Review Request 45855: Updated docs to include references to GPUs as a first class resource.

2016-04-08 Thread Ben Mahler
: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45855/ > --- > > (Updated April 7, 2016, 1:54 a.m.) > > >

Re: Review Request 45855: Updated docs to include references to GPUs as a first class resource.

2016-04-08 Thread Ben Mahler
he.org/r/45855/ > ------- > > (Updated April 7, 2016, 1:54 a.m.) > > > Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya. > > > Bugs: MESOS-5135 > https://issues.apache.org/jira/browse/MESOS-5135

Re: Review Request 45855: Updated docs to include references to GPUs as a first class resource.

2016-04-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45855/#review127889 --- Ship it! Ship It! - Ben Mahler On April 7, 2016, 1:54 a.m

Re: Review Request 45854: Updated the default JSON representation of a Resource to include GPUs.

2016-04-08 Thread Ben Mahler
) <https://reviews.apache.org/r/45854/#comment191248> I'm a little hesitant about using revocable gpus here given that it's not clear how it would work, but I suppose there is no harm in it for this test. - Ben Mahler On April 8, 2016, 5:54 a.m., K

Re: Review Request 45852: Added standard metrics for GPU resources.

2016-04-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45852/#review127886 --- Ship it! Ship It! - Ben Mahler On April 8, 2016, 8:34 p.m

Re: Review Request 45856: Fixed Nvidia GPU test build for namespace change of MasterDetector.

2016-04-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45856/#review127885 --- Ship it! Ship It! - Ben Mahler On April 7, 2016, 1:54 a.m

Re: Review Request 45853: Removed 'dashboard.js' from the webui.

2016-04-08 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45853/#review127860 --- Ship it! Ship It! - Ben Mahler On April 7, 2016, 1:54 a.m

Re: Review Request 44832: Validate string when convert `Flags` to `hashmap`.

2016-04-07 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44832/#review127727 --- Ship it! Ship It! - Ben Mahler On April 2, 2016, 8:07 a.m

Re: Review Request 45534: Added per-role and quota share metrics to the DRFSorter.

2016-04-07 Thread Ben Mahler
djacent with the expectation on the metrics? It's also how we did it in the metrics test above: ``` expected.values = { {"allocator/mesos/dominant_share/roles/roleA", 0.5}, }; metrics = Metrics(); EXPECT_T

Re: Review Request 44853: Added benchmark test for the allocator metrics endpoint.

2016-04-07 Thread Ben Mahler
watch.stop(); ``` src/tests/hierarchical_allocator_tests.cpp (lines 3029 - 3031) <https://reviews.apache.org/r/44853/#comment191091> How about: ``` cout << "/metrics/snapshot took " << watch.elapsed() << " for " <

Re: Review Request 45533: Explicitly typed quota role sorter in Mesos allocator.

2016-04-07 Thread Ben Mahler
/hierarchical.hpp (line 479) <https://reviews.apache.org/r/45533/#comment191095> whoops? - Ben Mahler On April 1, 2016, 8:07 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 45715: Added support to grant access to /dev/nvidiactl in Nvidia GPU isolator.

2016-04-05 Thread Ben Mahler
> On April 5, 2016, 11:53 p.m., Ben Mahler wrote: > > Do we also need the uvm device? - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45715/#rev

Re: Review Request 45715: Added support to grant access to /dev/nvidiactl in Nvidia GPU isolator.

2016-04-05 Thread Ben Mahler
/cgroups/devices/gpus/nvidia.cpp (lines 65 - 78) <https://reviews.apache.org/r/45715/#comment190468> Could we avoid the static non-POD? - Ben Mahler On April 4, 2016, 11:41 p.m., Kevin Klues wrote: > > --- > This is a

Re: Review Request 44364: Added tests for the Nvidia GPU isolator.

2016-04-05 Thread Ben Mahler
reviews.apache.org/r/44364/#comment190433> It's unfortunate that the curl filter used "error" to express existence, could we do a s/error/exists/ here? I'd also be ok shipping a patch for the curl filter if you want to take that on.

Re: Review Request 45622: Fixed Nvidia GPU isolator build for gcc.

2016-04-05 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45622/#review127231 --- Ship it! Ship It! - Ben Mahler On April 2, 2016, 9:35 p.m

Re: Review Request 45773: Updated the CHANGELOG for 0.28.1.

2016-04-05 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45773/#review127202 --- Ship it! Ship It! - Ben Mahler On April 5, 2016, 8:13 p.m

Re: Review Request 45439: Completed MVP implementation of the Nvidia GPU isolator.

2016-03-31 Thread Ben Mahler
op of the class and the commit message to mention the current caveats. - Ben Mahler On March 31, 2016, 4:48 p.m., Kevin Klues wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

Re: Review Request 44366: Added GPUs as an explicit resource.

2016-03-29 Thread Ben Mahler
nes 117 - 119) <https://reviews.apache.org/r/44366/#comment188913> We would probably need this TODO in all of our nvidia gpu blocks :) - Ben Mahler On March 14, 2016, 7:39 a.m., Kevin Klues wrote: > > --- > This is an auto

Re: Review Request 44365: Added flag to specify available Nvidia GPUs on an agent's command line.

2016-03-29 Thread Ben Mahler
- 1309) <https://reviews.apache.org/r/44365/#comment188908> Could we add something to clarify that this is relevant when "gpus" are specified within --resources? - Ben Mahler On March 14, 2016, 7:38 a.m., Kevin Klues wrote: > >

Re: Review Request 44361: Added configure flags to build with Nvidia GPU support.

2016-03-29 Thread Ben Mahler
y when the path is absolute? - Ben Mahler On March 14, 2016, 7:37 a.m., Kevin Klues wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http

Re: Review Request 44360: Added a script to install the Nvidia GDK on a host.

2016-03-29 Thread Ben Mahler
minor typos. - Ben Mahler On March 14, 2016, 7:35 a.m., Kevin Klues wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

Re: Review Request 44777: Added a flags parser for vector to src/common/parse.hpp.

2016-03-29 Thread Ben Mahler
: > https://reviews.apache.org/r/44777/ > ----------- > > (Updated March 18, 2016, 12:14 a.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-4926 > https://issues.apache.org/jira/b

Re: Review Request 44776: Did a general cleanup of s/.get()./->/ in the resources abstraction.

2016-03-29 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44776/#review125930 --- Ship it! Ship It! - Ben Mahler On March 14, 2016, 7:36 a.m

Re: Review Request 44974: Added device support in cgroups abstraction.

2016-03-28 Thread Ben Mahler
/#comment188662> I'm not sure if we need some of these comments, for example I can tell pretty easily from 'Access' that it represents the access permissions and it is part of a whitelist entry because it is contained within the ListEntry class. Ditto for selector. - Be

  1   2   3   4   5   6   7   8   9   10   >