Conversation
|
Build succeeded.
|
|
Thanks for the PR! Can you sign your commit? |
|
Do we need this for |
Only start watching the cgroup for OOMs when the first process starts instead of on every process. Signed-off-by: Seth Pellegrino <spellegrino@newrelic.com>
ea7cbf7 to
9456040
Compare
|
@estesp fixed! @AkihiroSuda It looks like it to me, though I'm not quite sure how to test the matrix of "v2 runtime with v1/v2 cgroups." That said, it looks like there's no OOM monitoring implemented for v2 cgroups, so I'm happy to move that if block down as part of this PR as well. |
|
Sorry I'm talking about cgroup v1 monitor in runc/v2. |
There's no OOM monitoring for the v2 cgroups yet, so it seems unlikely that there was a leak in that case. Signed-off-by: Seth Pellegrino <spellegrino@newrelic.com>
Codecov Report
@@ Coverage Diff @@
## master #3956 +/- ##
=======================================
Coverage 42.63% 42.63%
=======================================
Files 130 130
Lines 14759 14759
=======================================
Hits 6292 6292
Misses 7548 7548
Partials 919 919
Continue to review full report at Codecov.
|
estesp
left a comment
There was a problem hiding this comment.
LGTM
This looks like the right fix to me. Would love @crosbymichael's view, but he is out on leave for the time being.
|
Build succeeded.
|
|
Do you mind opening cherry-pick PR for release/1.3 branch? |
|
Sure thing: #3961 Thanks all! |
Only start watching the cgroup for OOMs when the first process starts instead of on every process.
In my testing this addresses the repro steps in #3949 and resolves the leak we were observing for kubernetes pods that have an exec healthcheck.