Added OOM notification for Memory controller by Zyqsempai · Pull Request #115 · containerd/cgroups

Zyqsempai · 2019-11-12T10:29:34Z

Partially Fixes #104

@crosbymichael @AkihiroSuda
So this Pr is on of the ideas I have on how we can implement OOM notifications in cGroupv2.
So, I see here at least two directions, we can implement Inotify fd check right here and in case if we receive inotify event, create new separate file for oom's only, and return that file fd.
On other hand we can delegate it to next layers(containerd etc.)
What do you think of this approach?
Signed-off-by: bpopovschi [email protected]

v2/manager.go

AkihiroSuda · 2019-11-12T10:37:50Z

in case if we receive inotify event, create new separate file for oom's only, and return that file fd.

Does kernel have this feature? Seems not https://facebookmicrosites.github.io/cgroup2/docs/memory-controller.html

v2/manager.go

Zyqsempai · 2019-11-12T12:56:42Z

@AkihiroSuda Do we actually care about other events here? Coz we can easily filter it only by oom

AkihiroSuda · 2019-11-12T12:59:16Z

Yes, but I agree it is not now

codecov-io · 2019-11-12T13:58:42Z

Codecov Report

Merging #115 into master will decrease coverage by 0.59%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master     #115     +/-   ##
=========================================
- Coverage   36.34%   35.75%   -0.6%     
=========================================
  Files          32       32             
  Lines        2102     2137     +35     
=========================================
  Hits          764      764             
- Misses       1210     1245     +35     
  Partials      128      128

Impacted Files	Coverage Δ
v2/manager.go	`0% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d26e1b3...d03d516. Read the comment docs.

Zyqsempai · 2019-11-12T13:59:11Z

@AkihiroSuda You mean something like this? PTAL

v2/manager.go

Zyqsempai · 2019-11-14T14:56:10Z

@crosbymichael PTAL

crosbymichael · 2019-11-14T16:14:44Z

rebase please :)

Signed-off-by: bpopovschi <[email protected]>

Zyqsempai · 2019-11-14T16:16:22Z

@crosbymichael Done.

crosbymichael · 2019-11-15T15:11:56Z

LGTM

AkihiroSuda reviewed Nov 12, 2019

View reviewed changes