mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alexander Rukletsov <ruklet...@gmail.com>
Subject Re: Review Request 63953: Added logging based on container class.
Date Mon, 27 Nov 2017 15:10:02 GMT

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




src/logging/utils.hpp
Lines 17-18 (patched)
<https://reviews.apache.org/r/63953/#comment269826>

    Can you please remind me why you create a separate file instead of adding it to "mesos/src/logging/logging.hpp"?



src/logging/utils.hpp
Lines 28-35 (patched)
<https://reviews.apache.org/r/63953/#comment269825>

    Maybe this capture the intention better?
    ```
    #define LOG_BASED_ON_CLASS(containerClass) \
      LOG_IF(INFO, (containerClass != ContainerClass::DEBUG) || VLOG_IS_ON(1))
    ```



src/slave/containerizer/mesos/containerizer.cpp
Lines 1928-1929 (original), 1932-1933 (patched)
<https://reviews.apache.org/r/63953/#comment269829>

    What about this?



src/slave/containerizer/mesos/containerizer.cpp
Lines 2322-2323 (original), 2326-2329 (patched)
<https://reviews.apache.org/r/63953/#comment269828>

    Let's add a utility function into `Container` struct. With a todo saying this function
will go away once `config` is not optional.



src/slave/containerizer/mesos/containerizer.cpp
Lines 2328 (patched)
<https://reviews.apache.org/r/63953/#comment269827>

    Option supports `operator->`



src/slave/containerizer/mesos/containerizer.cpp
Lines 2792-2795 (original), 2815-2818 (patched)
<https://reviews.apache.org/r/63953/#comment269830>

    What about this?


- Alexander Rukletsov


On Nov. 27, 2017, 2:16 p.m., Armand Grillet wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63953/
> -----------------------------------------------------------
> 
> (Updated Nov. 27, 2017, 2:16 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7361
>     https://issues.apache.org/jira/browse/MESOS-7361
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change adjusts log level based on the container class.
> If the class is `DEBUG` we print the log entry at a verbose
> level 1, otherwise we print it at the `INFO` level.
> 
> We use the added macro in mesos containerizer so that COMMAND
> checks cause less INFO logs (15 lines instead of 26 before).
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 9641ad4cf2a1f7fe56a4dcccb6eeff7130872fa8 
>   src/logging/utils.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/containerizer.cpp 7f3b86d87cf82429c2627d4a32eb0d5adbcc3f29

> 
> 
> Diff: https://reviews.apache.org/r/63953/diff/5/
> 
> 
> Testing
> -------
> 
> Started a Mesos cluster and used `mesos-execute` with this task group to check that the
behaviour after this patch is the one expected:
> 
> ```
> {
>   "tasks": [
>     {
>       "name": "Name of the task",
>       "task_id": {
>         "value": "task-group"
>       },
>       "agent_id": {
>         "value": ""
>       },
>       "resources": [
>         {
>           "name": "cpus",
>           "type": "SCALAR",
>           "scalar": {
>             "value": 0.01
>           }
>         },
>         {
>           "name": "mem",
>           "type": "SCALAR",
>           "scalar": {
>             "value": 2
>           }
>         }
>       ],
>       "command": {
>         "value": "sleep 1000"
>       },
>       "check": {
>         "type": "COMMAND",
>         "command": {
>           "command": {
>             "value": "echo \"Bonjour\""
>           },
>           "uris": []
>         }
>       }
>     }
>   ]
> }
> ```
> 
> And:
> ```
> $ nice make check
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>


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