subject:"Review Request 49851\: Implemented `MemorySubsystem`."

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-09-05 Thread haosdent huang



> On Aug. 22, 2016, 3:09 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp, 
> > lines 166-169
> > 
> >
> > 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?
> 
> Qian Zhang wrote:
> Jie, I remembered that we discussed this issue before, currently we don't 
> support restarting agent with a different set of isolators. So I think this 
> is a general issue for all isolators rather than specific to cgroups 
> isolator, maybe we need a dedicated project to handle it for all isolators?
> 
> Jie Yu wrote:
> Yeah, but that means we cannot safely add more cgroups systems without 
> reboot the box. That's really unfortunate.
> 
> haosdent huang wrote:
> Let me file a ticket and submit follow up patches to fix this.
> 
> haosdent huang wrote:
> When restart Mesos Agent with different cgroups subystems, current 
> unified cgroups isolator behaviour are same with the old one. They ignore it 
> in recover and continue to call `usage` and `status` on them. Because we 
> ignore failure in `usage` and `status`, restart Mesos Agent with different 
> cgoups subsystems is allowed. 
> 
> I create a ticket here https://issues.apache.org/jira/browse/MESOS-6063 
> to track this.
> 
> Jie Yu wrote:
> what about 'update', 'update' failures won't be ingored. I think this is 
> a pretty high priority issue. Could you please help solve it asap? I think 
> this will be a blocker if we want to turn on other subsystem support in a 
> backwards compatible way.

The link of the follow up patch is https://reviews.apache.org/r/51631/


- haosdent


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-09-04 Thread Jie Yu



> On Aug. 22, 2016, 3:09 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp, 
> > lines 166-169
> > 
> >
> > 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?
> 
> Qian Zhang wrote:
> Jie, I remembered that we discussed this issue before, currently we don't 
> support restarting agent with a different set of isolators. So I think this 
> is a general issue for all isolators rather than specific to cgroups 
> isolator, maybe we need a dedicated project to handle it for all isolators?
> 
> Jie Yu wrote:
> Yeah, but that means we cannot safely add more cgroups systems without 
> reboot the box. That's really unfortunate.
> 
> haosdent huang wrote:
> Let me file a ticket and submit follow up patches to fix this.
> 
> haosdent huang wrote:
> When restart Mesos Agent with different cgroups subystems, current 
> unified cgroups isolator behaviour are same with the old one. They ignore it 
> in recover and continue to call `usage` and `status` on them. Because we 
> ignore failure in `usage` and `status`, restart Mesos Agent with different 
> cgoups subsystems is allowed. 
> 
> I create a ticket here https://issues.apache.org/jira/browse/MESOS-6063 
> to track this.

what about 'update', 'update' failures won't be ingored. I think this is a 
pretty high priority issue. Could you please help solve it asap? I think this 
will be a blocker if we want to turn on other subsystem support in a backwards 
compatible way.


- Jie


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-22 Thread haosdent huang



> On Aug. 22, 2016, 3:09 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp, 
> > lines 166-169
> > 
> >
> > 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?
> 
> Qian Zhang wrote:
> Jie, I remembered that we discussed this issue before, currently we don't 
> support restarting agent with a different set of isolators. So I think this 
> is a general issue for all isolators rather than specific to cgroups 
> isolator, maybe we need a dedicated project to handle it for all isolators?
> 
> Jie Yu wrote:
> Yeah, but that means we cannot safely add more cgroups systems without 
> reboot the box. That's really unfortunate.
> 
> haosdent huang wrote:
> Let me file a ticket and submit follow up patches to fix this.

When restart Mesos Agent with different cgroups subystems, current unified 
cgroups isolator behaviour are same with the old one. They ignore it in recover 
and continue to call `usage` and `status` on them. Because we ignore failure in 
`usage` and `status`, restart Mesos Agent with different cgoups subsystems is 
allowed. 

I create a ticket here https://issues.apache.org/jira/browse/MESOS-6063 to 
track this.


- haosdent


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-22 Thread haosdent huang



> On Aug. 22, 2016, 3:04 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 553-559
> > 
> >
> > I think these codes can be merged into a single line 
> > `infos[containerId]->limiation.set(future);` which will internally handle 
> > all the cases (ready, failed, discarded).
> 
> haosdent huang wrote:
> +1

Fixed at https://reviews.apache.org/r/51294/


- haosdent


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-22 Thread haosdent huang



> On Aug. 22, 2016, 2:52 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp, line 112
> > 
> >
> > This comment needs to be updated.
> 
> haosdent huang wrote:
> My bad, let me fix it.

Fixed at https://reviews.apache.org/r/51293/


- haosdent


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-22 Thread haosdent huang



> On Aug. 22, 2016, 2:52 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp, line 112
> > 
> >
> > This comment needs to be updated.

My bad, let me fix it.


- haosdent


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-22 Thread haosdent huang



> On Aug. 22, 2016, 3:09 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp, 
> > lines 166-169
> > 
> >
> > 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?
> 
> Qian Zhang wrote:
> Jie, I remembered that we discussed this issue before, currently we don't 
> support restarting agent with a different set of isolators. So I think this 
> is a general issue for all isolators rather than specific to cgroups 
> isolator, maybe we need a dedicated project to handle it for all isolators?
> 
> Jie Yu wrote:
> Yeah, but that means we cannot safely add more cgroups systems without 
> reboot the box. That's really unfortunate.

Let me file a ticket and submit follow up patches to fix this.


> On Aug. 22, 2016, 3:09 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp, 
> > lines 54-58
> > 
> >
> > We avoid non-POD global variables. Why the change here?

We try to make it consistent with `DevicesSubsystem`, refer to 
https://reviews.apache.org/r/49854/#comment211199.


- haosdent


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-22 Thread haosdent huang



> On Aug. 22, 2016, 3:04 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 553-559
> > 
> >
> > I think these codes can be merged into a single line 
> > `infos[containerId]->limiation.set(future);` which will internally handle 
> > all the cases (ready, failed, discarded).

+1


- haosdent


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-21 Thread Jie Yu



> On Aug. 22, 2016, 3:09 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp, 
> > lines 166-169
> > 
> >
> > 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?
> 
> Qian Zhang wrote:
> Jie, I remembered that we discussed this issue before, currently we don't 
> support restarting agent with a different set of isolators. So I think this 
> is a general issue for all isolators rather than specific to cgroups 
> isolator, maybe we need a dedicated project to handle it for all isolators?

Yeah, but that means we cannot safely add more cgroups systems without reboot 
the box. That's really unfortunate.


- Jie


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


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-21 Thread Qian Zhang



> On Aug. 22, 2016, 11:09 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp, 
> > lines 166-169
> > 
> >
> > 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, I remembered that we discussed this issue before, currently we don't 
support restarting agent with a different set of isolators. So I think this is 
a general issue for all isolators rather than specific to cgroups isolator, 
maybe we need a dedicated project to handle it for all isolators?


- Qian


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


On Aug. 21, 2016, 5:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 21, 2016, 5:29 p.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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-21 Thread Jie Yu


---
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)


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



src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp (lines 
166 - 169)


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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-21 Thread Qian Zhang


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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 553 - 559)


I think these codes can be merged into a single line 
`infos[containerId]->limiation.set(future);` which will internally handle all 
the cases (ready, failed, discarded).


- Qian Zhang


On Aug. 21, 2016, 5:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 21, 2016, 5:29 p.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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-21 Thread Qian Zhang


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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 100)


This comment needs to be updated.


- Qian Zhang


On Aug. 21, 2016, 5:29 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 21, 2016, 5:29 p.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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-21 Thread haosdent huang


---
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.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  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

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-21 Thread haosdent huang


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

(Updated Aug. 21, 2016, 9:13 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @jieyu's comments.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  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

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-20 Thread Jie Yu


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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 142 - 146)


Hum, i don't like this approach. Can you introduce a `Subsystem::watch` 
method:

```
Future Subsystem::watch(
const ContainerID& containerId);
```

In the cgroups isolator `watch` method, simply invoke `subsystem->watch`. 

```
subsystem->watch(containerId)
  .onAny(defer(self(), ::_watch, ...));
  
void CgroupsIsolatorProcses::_watch(...)
{
  if (!infos.contains(containerId)) {
return;
  }
  
  infos[containerId]->limiation.set(limitation);
}
```


- Jie Yu


On Aug. 20, 2016, 9:20 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 20, 2016, 9:20 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 a83dde9f51e5601d02be6c233b1fcd5bee324ba1 
>   src/Makefile.am bfda83d112149cd3e5c579a33252e463212a9c5b 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 9b2d33ec3b023058d00c6671464cc9cd092f653b 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> cae642a8af8e45fdaa868826f48b9abcfd9bbd7f 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> cc402b4d5437c085e7fef1ff8207b5032c779e1e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 06f4400966ba467623556901b38d12e69fbbbd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 767c56d8fc8717d6a35bc5483d8b12b866d85e21 
>   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
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-20 Thread haosdent huang


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

(Updated Aug. 20, 2016, 9:20 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt a83dde9f51e5601d02be6c233b1fcd5bee324ba1 
  src/Makefile.am bfda83d112149cd3e5c579a33252e463212a9c5b 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
9b2d33ec3b023058d00c6671464cc9cd092f653b 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
cae642a8af8e45fdaa868826f48b9abcfd9bbd7f 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
cc402b4d5437c085e7fef1ff8207b5032c779e1e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
06f4400966ba467623556901b38d12e69fbbbd04 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
767c56d8fc8717d6a35bc5483d8b12b866d85e21 
  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

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-18 Thread haosdent huang


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

(Updated Aug. 18, 2016, 4:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
9b2d33ec3b023058d00c6671464cc9cd092f653b 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  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

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-17 Thread haosdent huang


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

(Updated Aug. 17, 2016, 5:43 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
9b2d33ec3b023058d00c6671464cc9cd092f653b 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  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

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-15 Thread haosdent huang


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

(Updated Aug. 15, 2016, 1:09 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
9b2d33ec3b023058d00c6671464cc9cd092f653b 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  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

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-14 Thread haosdent huang


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

(Updated Aug. 15, 2016, 3:20 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qian's comments.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/CMakeLists.txt 4362d472707193aa09343ef69f070e2d3efda324 
  src/Makefile.am d389e02584cfb1a00067cd30de1364118d1c46c7 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
9b2d33ec3b023058d00c6671464cc9cd092f653b 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
7e205a3eea4ad89faccf564bdb2ad0041fdee249 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
  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

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-13 Thread Qian Zhang


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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp (lines 69 - 73)


I would suggest to move this method right after the `cleanup()` method.


- Qian Zhang


On Aug. 11, 2016, 10:19 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 11, 2016, 10:19 p.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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> b191b2a52a9645fc902a35ed52909b2142f0b4c0 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2659252d8cffcefc233bc85fb4707c8147272737 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> c45d88092f3fe497373dfeaa8346aef9126c7b8b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-11 Thread haosdent huang


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

(Updated Aug. 11, 2016, 2:19 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-10 Thread haosdent huang


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

(Updated Aug. 10, 2016, 4:32 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address?@qianxhang's comment


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-09 Thread haosdent huang


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

(Updated Aug. 9, 2016, 6:40 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comment.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-07 Thread Qian Zhang



> On July 27, 2016, 10:05 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 494
> > 
> >
> > How do we recover this field `updatedLimit`? I mean during agent 
> > recovery, `updatedLimit` of each Info will be reset to `false`, right? But 
> > I think that is not correct.
> 
> haosdent huang wrote:
> I think this problem exists at `mem.cpp` as well, and update the limit 
> twice here does not bring problems here. So I drop this. Feel free to reopen 
> this if you think it would bring problems here.
> 
> Qian Zhang wrote:
> Yes, it is also a bug in `mem.cpp` which uses `info->pid` to check if it 
> is the first time to update the memory limit, but the value of `info->pid` 
> will also be lost during agent recovery.
> 
> And I think updating the memory hard limit twice will bring some 
> problems, as mentioned in this comments 
> (https://github.com/apache/mesos/blob/1.0.0/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L369:#L371),
>  decreasing memory hard limit may induce an OOM.
> 
> I had a discussion with Jie on this issue, and his suggestion is that we 
> should not use such kind of field (e.g., `pid` or `updatedLimit`) because 
> they can not be recovered, instead, before updating the hard limit, we should 
> check if its current value is still the initial value (should be -1? And it 
> may be different in different Linux distributions, so you may need to double 
> check it), if it is, then that means it is the first time to update the hard 
> limit, then we can just update it, if it is not, then that means it is not 
> the first time, then we can only increase it but not decrease it.
> 
> haosdent huang wrote:
> Thank you for your help. The initial value of limit_in_bytes is -1 
> (unlimited). Let's update to check by this.
> 
> haosdent huang wrote:
> Refer to http://lxr.free-electrons.com/source/kernel/res_counter.c?v=3.10 
> , http://lxr.free-electrons.com/source/kernel/res_counter.c?v=3.12 and 
> http://lxr.free-electrons.com/source/include/linux/page_counter.h 
> 
> There are two possible value of the initail values of `limit_in_bytes`: 
> 9223372036854775807 (LONG_MAX) and 18446744073709551615 (ULONG_MAX).

I see the third value of it in Ubuntu 16.04:
```
cat /sys/fs/cgroup/memory/memory.limit_in_bytes 
 
9223372036854771712
```
We need to cover it as well?


- Qian


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


On Aug. 7, 2016, 12:26 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 7, 2016, 12:26 p.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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> b191b2a52a9645fc902a35ed52909b2142f0b4c0 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2659252d8cffcefc233bc85fb4707c8147272737 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> c45d88092f3fe497373dfeaa8346aef9126c7b8b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-06 Thread haosdent huang


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

(Updated Aug. 7, 2016, 4:26 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comment.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-06 Thread Qian Zhang


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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 172)


s/uses/used/


- Qian Zhang


On Aug. 4, 2016, 1:58 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 4, 2016, 1:58 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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> b191b2a52a9645fc902a35ed52909b2142f0b4c0 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2659252d8cffcefc233bc85fb4707c8147272737 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> c45d88092f3fe497373dfeaa8346aef9126c7b8b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-04 Thread haosdent huang



> On July 27, 2016, 2:05 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 494
> > 
> >
> > How do we recover this field `updatedLimit`? I mean during agent 
> > recovery, `updatedLimit` of each Info will be reset to `false`, right? But 
> > I think that is not correct.
> 
> haosdent huang wrote:
> I think this problem exists at `mem.cpp` as well, and update the limit 
> twice here does not bring problems here. So I drop this. Feel free to reopen 
> this if you think it would bring problems here.
> 
> Qian Zhang wrote:
> Yes, it is also a bug in `mem.cpp` which uses `info->pid` to check if it 
> is the first time to update the memory limit, but the value of `info->pid` 
> will also be lost during agent recovery.
> 
> And I think updating the memory hard limit twice will bring some 
> problems, as mentioned in this comments 
> (https://github.com/apache/mesos/blob/1.0.0/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L369:#L371),
>  decreasing memory hard limit may induce an OOM.
> 
> I had a discussion with Jie on this issue, and his suggestion is that we 
> should not use such kind of field (e.g., `pid` or `updatedLimit`) because 
> they can not be recovered, instead, before updating the hard limit, we should 
> check if its current value is still the initial value (should be -1? And it 
> may be different in different Linux distributions, so you may need to double 
> check it), if it is, then that means it is the first time to update the hard 
> limit, then we can just update it, if it is not, then that means it is not 
> the first time, then we can only increase it but not decrease it.

Thank you for your help. The initial value of limit_in_bytes is -1 (unlimited). 
Let's update to check by this.


- haosdent


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


On Aug. 3, 2016, 5:58 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 3, 2016, 5:58 p.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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> b191b2a52a9645fc902a35ed52909b2142f0b4c0 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2659252d8cffcefc233bc85fb4707c8147272737 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> c45d88092f3fe497373dfeaa8346aef9126c7b8b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-03 Thread Qian Zhang

> On July 27, 2016, 10:05 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 494
> > 
> >
> > How do we recover this field `updatedLimit`? I mean during agent 
> > recovery, `updatedLimit` of each Info will be reset to `false`, right? But 
> > I think that is not correct.
> 
> haosdent huang wrote:
> I think this problem exists at `mem.cpp` as well, and update the limit 
> twice here does not bring problems here. So I drop this. Feel free to reopen 
> this if you think it would bring problems here.

Yes, it is also a bug in `mem.cpp` which uses `info->pid` to check if it is the 
first time to update the memory limit, but the value of `info->pid` will also 
be lost during agent recovery.

And I think updating the memory hard limit twice will bring some problems, as 
mentioned in this comments 
(https://github.com/apache/mesos/blob/1.0.0/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L369:#L371),
 decreasing memory hard limit may induce an OOM.

I had a discussion with Jie on this issue, and his suggestion is that we should 
not use such kind of field (e.g., `pid` or `updatedLimit`) because they can not 
be recovered, instead, before updating the hard limit, we should check if its 
current value is still the initial value (should be -1? And it may be different 
in different Linux distributions, so you may need to double check it), if it 
is, then that means it is the first time to update the hard limit, then we can 
just update it, if it is not, then that means it is not the first time, then we 
can only increase it but not decrease it.

- Qian

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

On Aug. 4, 2016, 1:58 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 4, 2016, 1:58 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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> b191b2a52a9645fc902a35ed52909b2142f0b4c0 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 2659252d8cffcefc233bc85fb4707c8147272737 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> c45d88092f3fe497373dfeaa8346aef9126c7b8b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-03 Thread haosdent huang


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

(Updated Aug. 3, 2016, 5:58 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Add process id.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-03 Thread haosdent huang


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

(Updated Aug. 3, 2016, 5:16 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
2659252d8cffcefc233bc85fb4707c8147272737 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-02 Thread haosdent huang



> On July 27, 2016, 2:05 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 494
> > 
> >
> > How do we recover this field `updatedLimit`? I mean during agent 
> > recovery, `updatedLimit` of each Info will be reset to `false`, right? But 
> > I think that is not correct.

I think this problem exists at `mem.cpp` as well, and update the limit twice 
here does not bring problems here. So I drop this. Feel free to reopen this if 
you think it would bring problems here.


- haosdent


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


On Aug. 2, 2016, 5:48 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated Aug. 2, 2016, 5:48 p.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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> b191b2a52a9645fc902a35ed52909b2142f0b4c0 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 179eb070d9621348bd7fa7d9a093a857ff0f2855 
>   src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
> c45d88092f3fe497373dfeaa8346aef9126c7b8b 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-02 Thread haosdent huang


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

(Updated Aug. 2, 2016, 5:48 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
b191b2a52a9645fc902a35ed52909b2142f0b4c0 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
179eb070d9621348bd7fa7d9a093a857ff0f2855 
  src/slave/containerizer/mesos/isolators/cgroups/constants.hpp 
c45d88092f3fe497373dfeaa8346aef9126c7b8b 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-08-01 Thread haosdent huang


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

(Updated Aug. 2, 2016, 3:52 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comment.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
422b4653fc3cb3d14c94b93ff456625fc59fbb27 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
bd20631a9650cf84e99c6489b2e92bc40ed764ca 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-31 Thread haosdent huang


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

(Updated July 31, 2016, 5:49 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
422b4653fc3cb3d14c94b93ff456625fc59fbb27 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
bd20631a9650cf84e99c6489b2e92bc40ed764ca 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-28 Thread Qian Zhang


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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 532 - 556)


I see you have changed the implementation of `usage()` and `_usage()` which 
are actually slightly different from the counterparts in the existing cgroups 
mem isolator, can you please elaborate why? And can we just keep consistent 
with cgroups mem isolator?


- Qian Zhang


On July 27, 2016, 1:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 27, 2016, 1:13 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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-26 Thread Qian Zhang


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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp (lines 71 - 75)


I think this should not be a virtual method, and it may be better to put it 
after the `cleanup()` method.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 81)


This comment needs to be updated since now the parameter is 
`_notifyCallback` rather than `_isolator`.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 83)


Does it have to be a virtual method?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 174)


s/that uses/that is used/



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 391)


I think here you should use `limitSwap` rather than 
`flags.cgroups_limit_swap`.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 494)


How do we recover this field `updatedLimit`? I mean during agent recovery, 
`updatedLimit` of each Info will be reset to `false`, right? But I think that 
is not correct.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 683)


I think you also need to do `infos.erase(containerId);` here.


- Qian Zhang


On July 27, 2016, 1:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 27, 2016, 1:13 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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-26 Thread haosdent huang



> On July 26, 2016, 1:40 a.m., haosdent huang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 498
> > 
> >
> > ```
> > if (info->updatedLimit || limit > currentLimit.get()) {
> > ```
> 
> Qian Zhang wrote:
> Can you check `info->pid.isNone()` just like what the existing cgroups 
> memory isolator is doing?

Because we didn't store `pid` in `info` here. Another reason is I think use 
`updatedLimit` would more clear.


- haosdent


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


On July 26, 2016, 5:13 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 26, 2016, 5:13 p.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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-26 Thread haosdent huang


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

(Updated July 26, 2016, 5:13 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address comments.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
348f105f9c3109a02f1dde0649f1b829cb9ddd04 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-26 Thread Qian Zhang



> On July 26, 2016, 9:40 a.m., haosdent huang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, line 498
> > 
> >
> > ```
> > if (info->updatedLimit || limit > currentLimit.get()) {
> > ```

Can you check `info->pid.isNone()` just like what the existing cgroups memory 
isolator is doing?


- Qian


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


On July 22, 2016, 3:10 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 22, 2016, 3:10 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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-25 Thread haosdent huang


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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 555)


Should be `await`


- haosdent huang


On July 21, 2016, 7:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 21, 2016, 7:10 p.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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-25 Thread haosdent huang


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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 436)


Check if `infos.contains(containerId)`;



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 498)


```
if (info->updatedLimit || limit > currentLimit.get()) {
```


- haosdent huang


On July 21, 2016, 7:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 21, 2016, 7:10 p.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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-25 Thread haosdent huang


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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (lines 79 - 88)


This part got
```
hides overloaded virtual function [-Woverloaded-virtual]
```
compile error in clang. Need to replace it with a no conflict name.


- haosdent huang


On July 21, 2016, 7:10 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 21, 2016, 7:10 p.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/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-21 Thread haosdent huang


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

(Updated July 21, 2016, 7:10 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-14 Thread haosdent huang


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

(Updated July 15, 2016, 3:33 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-14 Thread haosdent huang


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

(Updated July 14, 2016, 5:29 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Rebase.


Bugs: MESOS-5045
https://issues.apache.org/jira/browse/MESOS-5045


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
f677da3035181855f6490e3e516d13963ec43b51 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
64fca3c8776248fcbf2d5605966db60952149b2f 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-09 Thread haosdent huang


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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 559)


Kill this.


- haosdent huang


On July 9, 2016, 10:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 9, 2016, 10:18 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `MemorySubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49851: Implemented `MemorySubsystem`.

2016-07-09 Thread haosdent huang


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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 38)


Order incorrect.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 49)


Order incorrect.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 801)


Kill this.


- haosdent huang


On July 9, 2016, 10:18 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 9, 2016, 10:18 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `MemorySubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 49851: Implemented `MemorySubsystem`.

2016-07-09 Thread haosdent huang


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

Review request for mesos, Gilbert Song, Jie Yu, and Qian Zhang.


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

Diff: https://reviews.apache.org/r/49851/diff/


Testing
---


Thanks,

haosdent huang

50 matches

Mail list logo