mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevin Klues <klue...@gmail.com>
Subject Re: Review Request 48671: Restored ordering of --isolation entry processsing.
Date Tue, 14 Jun 2016 01:38:42 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/48671/#review137438
-----------------------------------------------------------


Fix it, then Ship it!




Looks good overall, just some small nits!


src/slave/containerizer/mesos/containerizer.cpp (line 32)
<https://reviews.apache.org/r/48671/#comment202563>

    You can probably safely remove the `#include` for `<utility>` and the `using std::pair`
as they were only introduced for the recent ordering changes that are now being reverted.



src/slave/containerizer/mesos/containerizer.cpp (line 347)
<https://reviews.apache.org/r/48671/#comment202565>

    You probably don't want to print `+ isolation +` here as it will double print below.


- Kevin Klues


On June 14, 2016, 1:31 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48671/
> -----------------------------------------------------------
> 
> (Updated June 14, 2016, 1:31 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-5581
>     https://issues.apache.org/jira/browse/MESOS-5581
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> In 6fb9c024, the ordering of isolators was changed from being
> specified by the operator to being specified by the code. It
> turns out that the operator ordering was intended to ensure
> that isolators can check their ordering requirements. If an
> isolator detects that its ordering requirements are not
> met, it can inform the operator to adjust the --isolation flag
> via a failure message.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp e5a339d965aeef2e3289c0ec3d9fb6eb281a567c

>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp d7557a0c338e8c0e51461b2326600c03f89c2e8b

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message