Re: Review Request 32750: MESOS-2585: Use full width for mesos div.container

+michael, does this look good to you? If so, I can get this committed. On Wed, Jun 24, 2015 at 12:35 AM, haosdent huang haosd...@gmail.com wrote: --- This is an automatically generated e-mail. To reply, visit:

Re: Review Request 34943: Added validation to flags.

Wasn't sure if you guys saw this? On Wed, Jun 24, 2015 at 3:22 PM, Ben Mahler benjamin.mah...@gmail.com wrote: This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34943/ 3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp

Re: Review Request 36929: Fixed a few issues in test launcher header.

Jie, I thought that duplicate includes of headers don't have a significant impact on compile times given our include guards, why do you say it slows down the compilation? e.g. https://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html On Thu, Jul 30, 2015 at 12:57 PM, Vinod Kone

Re: Review Request 36929: Fixed a few issues in test launcher header.

Well, it does seem easier to maintain includes if we rely on the parent header demonstrating intent to provide symbols (e.g. adding a vector to an interface does not require adding includes in all child files). If it provides significant speedup to build times, it would be very compelling! How

Re: Review Request 36425: Enabling IP Discovery script

https://issues.apache.org/jira/browse/MESOS-2902 says this will be done in a module instead? Do we still need this review? On Wed, Jul 22, 2015 at 3:31 AM, Mesos ReviewBot reviews@mesos.apache.org wrote: --- This is an automatically

Re: Review Request 39005: stout: Added thread-safe replacement for strerror.

Seems like os::strerror() would be more consistent with our other posix api wrappers. On Wed, Oct 7, 2015 at 9:27 AM, Bernd Mathiske wrote: > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39005/ >

Re: Review Request 37024: Exposes mesos version information in components.

Apologies, I've committed the fix for cmake. Looking forward to getting it set up in jenkins to catch these! On Sun, Oct 11, 2015 at 9:53 AM, haosdent huang wrote: > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/37024/ > > On

Re: Review Request 39014: RegistryClient refactor: renamed ManifestResponse

This patch still depends on https://reviews.apache.org/r/38579/, are you planning to remove the dependency? I can't ship these smaller changes since they depend on the large refactor and your layer id patch: https://reviews.apache.org/r/38443/ On Tue, Oct 6, 2015 at 5:16 PM, Jojy Varghese

Re: Review Request 36180: Avoid multi writers write to same file in PortMappingIsolatorTests.

Jie or Ian, can you shepherd this? On Sat, Oct 3, 2015 at 5:25 PM, Mesos ReviewBot wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36180/#review101422 >

Review Request 48426: Made logging of Subprocess errors consistent in Docker wrapper.

--- See summary. Diffs - src/docker/docker.cpp ffbd7ef56d02dfe550a60851251e6645d2ca5925 Diff: https://reviews.apache.org/r/48426/diff/ Testing --- make check Thanks, Benjamin Mahler

Review Request 48427: Avoid killing a stale pid in the docker/command executors.

1b3a7795b1db83394d4b884c1041c341f88a7df1 src/launcher/executor.cpp ffebb3a496be6a20396f8f539372a1e7527c9b6d Diff: https://reviews.apache.org/r/48427/diff/ Testing --- make check Thanks, Benjamin Mahler

Review Request 48424: Avoid use of CHECK within Docker tests.

--- See summary. Diffs - src/tests/containerizer/docker_tests.cpp 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f Diff: https://reviews.apache.org/r/48424/diff/ Testing --- sudo make check Thanks, Benjamin Mahler

Review Request 48423: Simplified the test for Docker::version.

--- This test assumes that a Try is Some, and the test can be written more succinctly. Diffs - src/tests/containerizer/docker_tests.cpp 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f Diff: https://reviews.apache.org/r/48423/diff/ Testing --- sudo make check Thanks, Benjamin Mahler

Review Request 48422: Removed an unused function in src/docker/docker.cpp.

--- See summary. Diffs - src/docker/docker.cpp ffbd7ef56d02dfe550a60851251e6645d2ca5925 Diff: https://reviews.apache.org/r/48422/diff/ Testing --- make check Thanks, Benjamin Mahler

Review Request 48428: Updated Docker::run to return exit status of container.

5591a6784afd10b4c7535f90c2e6745fa74c318b src/tests/containerizer/docker_tests.cpp 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f src/tests/mesos.hpp 6f2888023c957f0af4f7374c98e406816a8423e3 Diff: https://reviews.apache.org/r/48428/diff/ Testing --- sudo make check Thanks, Benjamin Mahler

Review Request 48430: Added Docker::kill.

d9ffc18496718701fac8182506a8c36e21e9c319 src/docker/docker.cpp ffbd7ef56d02dfe550a60851251e6645d2ca5925 src/tests/containerizer/docker_tests.cpp 7ef52ade0d3389f9e24e3c5c7dda4f8809b9d83f Diff: https://reviews.apache.org/r/48430/diff/ Testing --- Added a test. Thanks, Benjamin Mahler

Review Request 48429: Fixed broken docker logging redirection in the docker executor.

/ Testing --- sudo make check Thanks, Benjamin Mahler

Review Request 48425: Fixed confusing logging of WSTRINGIFY in Docker wrapper.

--- WSTRINGIFY will specify whether the command terminated with a particular singal or exited with a particular status. Diffs - src/docker/docker.cpp ffbd7ef56d02dfe550a60851251e6645d2ca5925 Diff: https://reviews.apache.org/r/48425/diff/ Testing --- make check Thanks, Benjamin

Re: Review Request 47719: Removed invalid gpu statistics columns in framework data tables.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47719/#review136730 --- Ship it! Thanks! - Benjamin Mahler On May 23, 2016, 6:11

Re: Review Request 43561: Improve Ranges parsing to handle single values.

ge what we accept. Ideally, we could pull in a regex or parsing expression grammar library to more formally define our formats. The current parsing code is a mess. - Benjamin Mahler On March 22, 2016, 1:22 p.m., Klaus Ma wrote: > > -

Re: Review Request 47362: Added a test for the SSL head-of-line blocking issue in MESOS-5340.

ly generated e-mail. To reply, visit: https://reviews.apache.org/r/47362/#review133213 --- On May 13, 2016, 9:46 p.m., Benjamin Mahler wrote: > > --- > This is an automatica

Re: Review Request 48478: Created a `cgroups/devices` isolator.

the new files to the cmake build. We should have some tests for this new isolator, I'll leave MESOS-5582 open for the addition of the tests. - Benjamin Mahler On June 9, 2016, 7:34 a.m., Kevin Klues wrote: > > --- > This is an auto

Re: Review Request 48477: Added ability to express dependencies between isolators.

isolators.push_back(Owned(isolator.get())); - types.erase(creator.first); + isolators.emplace_back(isolator.get()); } } - if (types.size() != 0) { -return Error("Unknown or unsupported isolators '" + stringify(types) + "'"); + i

Re: Review Request 48455: Removed unused import from sorter.cpp.

tps://reviews.apache.org/r/48455/#comment202002> Rather than just removing this one, would you mind doing an audit of the headers? We're missing a lot of includes here. - Benjamin Mahler On June 9, 2016, 1:30 a.m., Guangya Liu

Re: Review Request 48429: Fixed broken docker logging redirection in the docker executor.

36881 ------- On June 9, 2016, 4:04 a.m., Benjamin Mahler wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/

Re: Review Request 48458: Corrected comment for QuotaProvidesGuarantee.

> On June 9, 2016, 11:52 p.m., Benjamin Mahler wrote: > > Ship It! We should clarify that QuotaProvidesGuarantee is a test in the summary of this review because it will become the commit message. -

Re: Review Request 48458: Corrected comment for QuotaProvidesGuarantee.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48458/#review136934 --- Ship it! Ship It! - Benjamin Mahler On June 9, 2016, 2:38

Re: Review Request 48428: Updated Docker::run to return exit status of container.

This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48428/#review136726 --- On June 9, 2016, 3:57 a.m., Benjamin Mahler wrote: > > ---

Re: Review Request 47362: Added a test for the SSL head-of-line blocking issue in MESOS-5340.

the response due to the string construction. - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47362/#review133250 --- On M

Re: Review Request 47362: Added a test for the SSL head-of-line blocking issue in MESOS-5340.

the encoder. The response encoder requires the corresponding request object, which we don't have here (I'd have to make this test more inconsistent with the rest). - Benjamin --- This is an automatically generated e-mail.

Re: Review Request 48578: Fixed bug with double destruction of cgroups devices subsystem.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48578/#review137573 --- Ship it! Ship It! - Benjamin Mahler On June 11, 2016, 3:03

Re: Review Request 48370: Fixed method of populating device entries for `/dev/nvidia-uvm`, etc.

Ditto below. - Benjamin Mahler On June 11, 2016, 3:06 a.m., Kevin Klues wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https:

Re: Review Request 48363: Moved 'isolators/cgroups/devices/gpus' to 'isolators/gpu'.

(lines 59 - 63) <https://reviews.apache.org/r/48363/#comment202758> Looks like this should be alongside isolator includes? - Benjamin Mahler On June 11, 2016, 3:04 a.m., Kevin Klues wrote: > > --- > This is an automatica

Review Request 48662: Fixed a regression in the creation of the isolator modules.

/containerizer.cpp 92c9898fb41ca0ffbda05e53b595b05c96f4f596 Diff: https://reviews.apache.org/r/48662/diff/ Testing --- sudo make check Thanks, Benjamin Mahler

Re: Review Request 48422: Removed an unused function in src/docker/docker.cpp.

have lambdas to make this cleaner :) - Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48422/#review136712 --- On June 8, 2016, 7:18

Re: Review Request 48423: Simplified the test for Docker::version.

Benjamin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48423/#review136714 --- On June 8, 2016, 7:18 p.m., Benjamin Mahler wrote: > > --- > This i

Re: Review Request 48428: Updated Docker::run to return exit status of container.

6f2888023c957f0af4f7374c98e406816a8423e3 Diff: https://reviews.apache.org/r/48428/diff/ Testing --- sudo make check Thanks, Benjamin Mahler

Re: Review Request 48429: Fixed broken docker logging redirection in the docker executor.

) - src/docker/executor.cpp 1b3a7795b1db83394d4b884c1041c341f88a7df1 Diff: https://reviews.apache.org/r/48429/diff/ Testing --- sudo make check Thanks, Benjamin Mahler

Re: Review Request 47991: Passed `Request` object by ptr instead of copying it.

(line 1285) <https://reviews.apache.org/r/47991/#comment200669> We should add a comment here since this is a bit of a hack to avoid copying the request. Ideally we make it a Shared or Owned when it is received from the decoder. - Benjamin Mahler On May 27, 2016, 10:14 p.m., Anand Ma

Re: Review Request 47990: Added move semantics to `Pipe::write()`.

reader } else { moved = true; add to writes } ``` This seems to miss the idea? - Benjamin Mahler On May 30, 2016, 11:21 p.m., Anand Mazumdar wrote: > > --- > This is an automatically generated e

Review Request 48671: Restored ordering of --isolation entry processsing.

/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp d7557a0c338e8c0e51461b2326600c03f89c2e8b Diff: https://reviews.apache.org/r/48671/diff/ Testing --- make check Thanks, Benjamin Mahler

Re: Review Request 48906: Added helper function to get non-scarce resources.

> On June 21, 2016, 10:36 p.m., Benjamin Mahler wrote: > > include/mesos/resources.hpp, lines 252-256 > > <https://reviews.apache.org/r/48906/diff/1/?file=1423388#file1423388line252> > > > > The Resources abstraction shouldn't need to know anything about sc

Re: Review Request 48986: Updated the comments for GPU_RESOURCES.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48986/#review139000 --- Ship it! Ship It! - Benjamin Mahler On June 22, 2016, 12:07

Re: Review Request 49013: Updated cgroups/devices isolator's prepare() to directly return None().

> On June 21, 2016, 10:05 p.m., Benjamin Mahler wrote: > > Can you describe why you made this change? > > > > This follows the same pattern as other isolators (e.g. cpushare.cpp, > > mem.cpp), so it's hard to tell what the issue is here. > > Qian Zhang

Review Request 49057: Updated a CHECK in Subprocess to include the failure message.

--- See summary. Diffs - 3rdparty/libprocess/include/process/posix/subprocess.hpp 66efa257850e62cb18a40cac1820f4df37f2a114 Diff: https://reviews.apache.org/r/49057/diff/ Testing --- make check Thanks, Benjamin Mahler

Re: Review Request 49057: Updated a CHECK in Subprocess to include the failure message.

: https://reviews.apache.org/r/49057/diff/ Testing --- make check Thanks, Benjamin Mahler

Re: Review Request 48369: Fixed comment in Nvidia GPU device isolator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48369/#review137642 --- Ship it! Ship It! - Benjamin Mahler On June 11, 2016, 3:06

Re: Review Request 48368: Changed major/minor device types for Nvidia GPUs to `unsigned int`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48368/#review137641 --- Ship it! Ship It! - Benjamin Mahler On June 11, 2016, 3:06

Re: Review Request 48376: Changed semantics for granting access to /dev/nvidiactl, etc.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48376/#review138097 --- Ship it! - Benjamin Mahler On June 11, 2016, 3:05 a.m

Re: Review Request 48365: Bundled NVML headers for Nvidia GPU support.

e here? - Benjamin Mahler On June 11, 2016, 3:37 a.m., Kevin Klues wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 48367: Added test to verify that GPU auto-discovery works.

/nvidia_gpu_isolator_tests.cpp (lines 231 - 232) <https://reviews.apache.org/r/48367/#comment203325> Let's break this test apart so that it's a bit clearer what is being tested. For example: AutoDiscovery FlagValidation - Benjamin Mahler On June 11, 2016

Re: Review Request 48366: Added auto-discovery of GPUs for Nvidia GPU support.

we print out to the user (error messages, log messages, etc). src/slave/containerizer/containerizer.cpp (line 140) <https://reviews.apache.org/r/48366/#comment203320> s/unsigned/non-negative/? - Benjamin Mahler On June 11, 2016, 3:07 a.m.,

Re: Review Request 48832: Changed build to always enable Nvidia GPU support for Linux.

tps://reviews.apache.org/r/48832/#comment203458> Perhaps we need a comment here to point out that libelf is needed for the Nvidia GPU support? - Benjamin Mahler On June 17, 2016, 6:29 a.m., Kevin Klues wrote: > > --- > This is a

Re: Review Request 48906: Added helper function to get non-scarce resources.

tps://reviews.apache.org/r/48906/#comment204151> The Resources abstraction shouldn't need to know anything about scarce resources, you can just use the 'filter' function above if you want to perform some custom filtering in the caller. - Benjamin Mahler On June 18, 2016, 1:04 p.m., Guangya Liu

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

> On June 17, 2016, 10:07 p.m., Benjamin Mahler wrote: > > While it's nice to document that this is internally only doing a > > receive-side shutdown, I'm left wondering why. And now that it's > > documented, the restriction looks intentional but the intention is

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

behavior than BSD sockets, let me know if I'm missing something there. - Benjamin Mahler On June 20, 2016, 12:35 p.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 48986: Updated a typo for GPU_RESOURCES.

on GPU agents, in order to // avoid occupying a GPU agent and preventing GPU workloads // from running! Currently, if a ... ``` - Benjamin Mahler On June 21, 2016, 2:14 p.m., Guangya Liu

Re: Review Request 48579: Reset "dirty" to false in sort().

. For example, when the weights are updated, why aren't we setting dirty to true? src/master/allocator/sorter/drf/sorter.cpp (lines 343 - 345) <https://reviews.apache.org/r/48579/#comment204125> No need for the comment here. - Benjamin Mahler On June 11, 2016, 8:30 a.m., Guangya Liu

Re: Review Request 48062: Recalculated share when weight was updated.

; // shares: a = .05, b = .06 EXPECT_EQ(list({"a", "b"}), sorter.sort()); // Increase b's weight to flip the sort order. sorter.update("b", 2); // shares: a = .05, b = .03 EXPECT_EQ(

Re: Review Request 48579: Reset "dirty" to false in sort().

> On June 21, 2016, 8:35 p.m., Benjamin Mahler wrote: > > It looks like dirty is not being set to true in all cases. For example, > > when the weights are updated, why aren't we setting dirty to true? Just saw that you fixed this in your next patch, I'll go through this again

Re: Review Request 48455: Some cleanup for sorter.cpp.

t204122> Include for CHECK_SOME? src/master/allocator/sorter/drf/sorter.cpp (line 416) <https://reviews.apache.org/r/48455/#comment204124> Include for Option? - Benjamin Mahler On June 11, 2016, 8:27 a.m., Guangya Liu wrote: > > --

Re: Review Request 48910: Highlight disconnected frameworks in webui.

> On June 21, 2016, 9:32 p.m., Benjamin Mahler wrote: > > Very nice, however the meaning of the color doesn't seem discoverable. Do > > we also need a legend or an explicit field to indicate the state? > > Tomasz Janiszewski wrote: > How about adding "Conn

Re: Review Request 48579: Reset "dirty" to false in sort().

plementation shifts. - Benjamin Mahler On June 11, 2016, 8:30 a.m., Guangya Liu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 48455: Some cleanup for sorter.cpp.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48455/#review138944 --- Ship it! - Benjamin Mahler On June 11, 2016, 8:27 a.m

Re: Review Request 49013: Updated cgroups/devices isolator's prepare() to directly return None().

pattern as other isolators (e.g. cpushare.cpp, mem.cpp), so it's hard to tell what the issue is here. - Benjamin Mahler On June 21, 2016, 8:43 a.m., Qian Zhang wrote: > > --- > This is an automatically generated e-mail. To rep

Re: Review Request 48910: Highlight disconnected frameworks in webui.

discoverable. Do we also need a legend or an explicit field to indicate the state? - Benjamin Mahler On June 18, 2016, 7:16 p.m., Tomasz Janiszewski wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 48911: Show orphan tasks in WebUI.

e pull this fix into a patch separate from this one. - Benjamin Mahler On June 18, 2016, 8:22 p.m., Tomasz Janiszewski wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 48911: Show orphan tasks in WebUI.

> On June 21, 2016, 9:39 p.m., Benjamin Mahler wrote: > > src/webui/master/static/js/controllers.js, lines 151-154 > > <https://reviews.apache.org/r/48911/diff/1/?file=1423454#file1423454line151> > > > > This still looks incorrect, we need to set the stopped

Re: Review Request 48885: Display used Agents resources in webui.

ages (e.g. agent_framework.html) have progress bars?) - Benjamin Mahler On June 17, 2016, 10:06 p.m., Tomasz Janiszewski wrote: > > --- > This is an automatically generated

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

`Socket::shutdown`? Can you check with Joris? [1] https://github.com/apache/mesos/blob/0.22.0/3rdparty/libprocess/src/process.cpp#L1772-L1775 - Benjamin Mahler On June 16, 2016, 8:55 a.m., Neil Conway wrote: > > --- >

Re: Review Request 48885: Display allocated Agents resources in webui.

/agents.html (lines 20 - 21) <https://reviews.apache.org/r/48885/#comment203522> Whoops, there should be no change to these two. - Benjamin Mahler On June 17, 2016, 10:37 p.m., Tomasz Janiszewski wrote: > > --- > This is a

Re: Review Request 48883: Fixed Cmake build for Nvidia GPU support on Linux in libprocess.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48883/#review138323 --- Ship it! Ship It! - Benjamin Mahler On June 17, 2016, 8:58

Re: Review Request 48885: Display allocated Agents resources in webui.

> On June 17, 2016, 10:19 p.m., Benjamin Mahler wrote: > > Very nice! > > Tomasz Janiszewski wrote: > @bmahler Thanks for review. I fixed issues. Would you mind take a look > once again. > BTW: I saw your > [todo](https://g

Review Request 49108: Cherry-pick of MESOS-5330 to 0.27.x.

/49108/diff/ Testing --- N/A Thanks, Benjamin Mahler

Review Request 49109: Cherry-pick of MESOS-5330 to 0.28.x.

/49109/diff/ Testing --- N/A Thanks, Benjamin Mahler

Re: Review Request 49013: Updated cgroups/devices isolator's prepare() to directly return None().

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49013/#review139149 --- Ship it! Ship It! - Benjamin Mahler On June 22, 2016, 9:51

Re: Review Request 48906: Added helper function to get non-scarce resources.

> On June 21, 2016, 10:36 p.m., Benjamin Mahler wrote: > > include/mesos/resources.hpp, lines 252-256 > > <https://reviews.apache.org/r/48906/diff/1/?file=1423388#file1423388line252> > > > > The Resources abstraction shouldn't need to know anything about sc

Re: Review Request 49127: Fix socket leak in SSL_ENABLE_DOWNGRADE code path.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49127/#review139200 --- Ship it! Ship It! - Benjamin Mahler On June 23, 2016, 2:07

Re: Review Request 48914: Added GPU_RESOURCES capability to FrameworkInfo.

able to use gpus, rather than just being aware of GPUs (e.g. be careful about running non-GPU workloads on GPU machines, in the future may experience more revocation). - Benjamin Mahler On June 18, 2016, 10:07 p.m., Kevin

Re: Review Request 48914: Added GPU_RESOURCES capability to FrameworkInfo.

-mail. To reply, visit: > https://reviews.apache.org/r/48914/ > --- > > (Updated June 18, 2016, 10:07 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-5634 > https:/

Re: Review Request 48915: Added an example framework for consuming GPUs.

upport for `--num_tasks`. In the past we found it was a burden to maintain a proliferation of example frameworks so it would be great if we could leverage the command (aka "no executor") framework here by adding the `--capabilities` flag. - Benjamin Mahler

Re: Review Request 48979: Added check for `--isolation=gpu/nvidia` in order to autodiscover GPUs.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48979/#review138734 --- Ship it! - Benjamin Mahler On June 21, 2016, 1:36 a.m

Re: Review Request 48914: Added GPU_RESOURCES capability to FrameworkInfo.

> On June 21, 2016, 1:23 a.m., Benjamin Mahler wrote: > > include/mesos/mesos.proto, lines 278-281 > > <https://reviews.apache.org/r/48914/diff/1/?file=1423477#file1423477line278> > > > > How about the following? > > > > ``` > >

Re: Review Request 48364: Removed hard dependence on `libnvidia-ml.so` for Nvidia GPU support.

log to obtain CHECK src/slave/containerizer/mesos/isolators/gpu/nvml.cpp (line 216) <https://reviews.apache.org/r/48364/#comment203019> To avoid double logging we should omit the caller available information here (the index). - Benjamin Mah

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46025/#review138320 --- Ship it! Ship It! - Benjamin Mahler On June 16, 2016, 8:55

Re: Review Request 48881: Fixed Cmake build for Nvidia GPU support on Linux.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48881/#review138321 --- Ship it! Ship It! - Benjamin Mahler On June 17, 2016, 8:57

Re: Review Request 48882: Fixed Cmake build for Nvidia GPU support on Linux in stout.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48882/#review138322 --- Ship it! Ship It! - Benjamin Mahler On June 17, 2016, 8:58

Re: Review Request 48571: Added assertion for initialization success to the log replica tests.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48571/#review138353 --- Ship it! Ship It! - Benjamin Mahler On June 15, 2016, 9:03

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

confusion, it seems to be only because there is a potential reason for only allowing `SHUT_RD`. Let's get context from benh and joris to figure that out. If it was intentional, let's include the reason in your comment, otherwise let's allow the caller to specify `how`. - Benjamin Mahler

Re: Review Request 42379: Add reverse_foreach in libprocess.

It seems unfortunate to introduce alternative looping constructs, for example the following would be the more composable approach: foreach (int i, reversed(numbers)) { } I remember this coming up before:

Re: Review Request 49175: Added tests for libprocess linking and unlinking behavior.

ver the UPID of the linkee. // // In order to simulate "stale" links (see MESOS-), this // will exit upon receiving a message. class LinkeeProcess ``` 3rdparty/libprocess/src/tests/test_linkee.cpp (lines 44 - 48) <https://revi

Re: Review Request 49177: Added 'relink' semantics to ProcessBase::link.

ame code in link_connect) in a separate patch (no need for to send a review), can do the following: ``` socket->connect(to.address) ``` - Benjamin Mahler On June 24, 2016, 2:43 a.m., Joseph Wu wrote: > > --- >

Re: Review Request 49174: Added test-only function for retrieving link sockets from libprocess.

(line 314) <https://reviews.apache.org/r/49174/#comment204977> How about making this const and using `.at()` instead of the `[]` operator? - Benjamin Mahler On June 24, 2016, 2:43 a.m., Joseph Wu wrote: > > --- > This is a

Re: Review Request 49280: Fixed FD inheritance leak when SSL is enabled.

ilure Future into the accept queue? (Much like how the poll-based socket works?) That way we can deal with the error in the accept loop in libprocess. - Benjamin Mahler On June 27, 2016, 7:38 p.m., Joseph Wu wrote: > > --- > This is an auto

Re: Review Request 48372: Updated `Containerizer::resources()` to use the `NvidiaGpuAllocator`.

ssume this is why you added the filter? ``` // When adding in the GPU resources, make sure that we filter out // the existing GPU resources (if any) so that we do not double // allocate GPUs. ``` - Benjamin Mahler On June

Re: Review Request 48373: Integrated the `NvidiaGpuAllocator` into the `NvidiaGpuIsolator`.

s.at(containerId); infos.erase(containerId); return Nothing(); ``` - Benjamin Mahler On June 23, 2016, 4:34 p.m., Kevin Klues wrote: > > --- > This is an automatically generated

Re: Review Request 48371: Added `NvidiaGpuAllocator` component.

``` src/tests/containerizer/nvidia_gpu_isolator_tests.cpp (lines 442 - 474) <https://reviews.apache.org/r/48371/#comment204647> No need for all of these result variables, we can `AWAIT_READY(f())` directly. - Benjamin Mahler On June

Re: Review Request 44379: Correctly parse perf stat format for non-vanilla 3.10 kernel.

Hey Fan, Left some comments, mostly I'm a bit confused by your comment that OS vendors can tweak the format. How did you know they only tweak it that particular way? Ben On Wed, Mar 9, 2016 at 11:53 PM, Mesos ReviewBot wrote: > This is an automatically generated

Re: Review Request 45096: Introduce 'minus' operator for class Counter.

Hey guys, we intentionally removed the ability to decrement Counters, please see the context here on another recent patch: https://reviews.apache.org/r/44473/ On Tue, Mar 22, 2016 at 12:00 AM, Benjamin Bannier < benjamin.bann...@mesosphere.io> wrote: > >

Review Request 47625: Fixed incorrect use of unique_ptr for array storage.

on. Thanks, Benjamin Mahler

  1   2   3   4   5   6   7   8   9   10   >