> On May 13, 2024, 7:42 p.m., Benjamin Mahler wrote: > > src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp > > Lines 281-299 (patched) > > <https://reviews.apache.org/r/74979/diff/1/?file=2287738#file2287738line281> > > > > I suspect we should just remove the stats from the message since we're > > reacting post-OOM, and the oom killer may have already killed processes. > > This makes the stats unreliable and users need to rely instead on the > > historical container usage metrics to better understand where the memory > > consumption was occurring. > > Jason Zhou wrote: > I will be removing all reported categories except Anon, File, and Kernel. > Will add a disclaimer on line 282 to state that the statistics are reported > post-oom for clarity. I will also remove the TODO on Line 297
Done > On May 13, 2024, 7:42 p.m., Benjamin Mahler wrote: > > src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp > > Lines 286 (patched) > > <https://reviews.apache.org/r/74979/diff/1/?file=2287738#file2287738line286> > > > > we should't return here, since we still want to report that the > > limitation was reached, similar to how our v1 isolation logic proceeds > > despite errors Done, program flow updated, we only print the stats when memoryStats.isError() returns false. > On May 13, 2024, 7:42 p.m., Benjamin Mahler wrote: > > src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp > > Lines 306 (patched) > > <https://reviews.apache.org/r/74979/diff/1/?file=2287738#file2287738line306> > > > > ditto here Done > On May 13, 2024, 7:42 p.m., Benjamin Mahler wrote: > > src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp > > Lines 311 (patched) > > <https://reviews.apache.org/r/74979/diff/1/?file=2287738#file2287738line311> > > > > ditto here Done > On May 13, 2024, 7:42 p.m., Benjamin Mahler wrote: > > src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp > > Lines 324-325 (patched) > > <https://reviews.apache.org/r/74979/diff/1/?file=2287738#file2287738line324> > > > > ditto here, need to handle missing hard limit Done, we now set the megabytes variable to 0 if hardLimit is missing. - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/74979/#review226435 ----------------------------------------------------------- On May 13, 2024, 9:59 p.m., Jason Zhou wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/74979/ > ----------------------------------------------------------- > > (Updated May 13, 2024, 9:59 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Introduces OOM listening to the MemoryControllerProcess so that we > detect, report, and respond to OOM events. > > > Diffs > ----- > > src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.hpp > 2e60b2c19a781c2d8ab24e89e440383ca517868c > src/slave/containerizer/mesos/isolators/cgroups2/controllers/memory.cpp > 732b1c65febdc78d8854e571bb02a9d367528434 > > > Diff: https://reviews.apache.org/r/74979/diff/3/ > > > Testing > ------- > > > Thanks, > > Jason Zhou > >
