mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 49851: Implemented `MemorySubsystem`.
Date Mon, 22 Aug 2016 03:09:47 GMT

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


Fix it, then Ship it!




I made some adjustments while committing. Please take a look.


src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp (lines 54 - 58)
<https://reviews.apache.org/r/49851/#comment212649>

    We avoid non-POD global variables. Why the change here?



src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp (lines 166 - 169)
<https://reviews.apache.org/r/49851/#comment212658>

    Realized an issue. What if the operator changes the flags to add memory cgroup support
later. During recovery, we don't recovery the container. That means the subsequent update
will fail later.
    
    I am wondering if we should add some logic in the cgroups isolator to remember the subsystems
that we recovered (also in launch path), and only call `update`, `wait`, `usage`, `status`
on those subsystems?
    
    Can you follow up on that?


- Jie Yu


On Aug. 21, 2016, 9:29 a.m., haosdent huang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> -----------------------------------------------------------
> 
> (Updated Aug. 21, 2016, 9:29 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5045
>     https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented `MemorySubsystem`.
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt ff51705202569b85922d877ef95750d7943372d8 
>   src/Makefile.am 61c941f42bdccdf1745f72f875cb5c5a9901dc76 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 9b2d33ec3b023058d00c6671464cc9cd092f653b

>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 0c724523a8b46f74a52214ae2b223f41f8dc2d58

>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 4a7b556bb74cfda0a74e1b9b5d4bb226b84751c1

>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 06f4400966ba467623556901b38d12e69fbbbd04

>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp df701d2fa7ce7622e6a32904ba3a99fd36b02df6

>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp PRE-CREATION

>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> haosdent huang
> 
>


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