You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2020/04/27 14:57:51 UTC
Review Request 72442: Reverted the changes about
`REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72442/
-----------------------------------------------------------
Review request for mesos, Andrei Budnik and Greg Mann.
Bugs: MESOS-10049
https://issues.apache.org/jira/browse/MESOS-10049
Repository: mesos
Description
-------
When a burstable container is OOM killed because the agent host is running
out of memory, the method `MemorySubsystemProcess::oomWaited` will NOT be
invoked, it will only be invoked when the container uses more memory than
its hard memory limit (i.e., `REASON_CONTAINER_LIMITATION_MEMORY`). So we
do not need to introduce `REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
Diffs
-----
include/mesos/mesos.proto 9412ed736231547b22abc89188316b08d5445e78
include/mesos/v1/mesos.proto 194c42cf57e34d803a21cab03db17614855e8692
src/common/protobuf_utils.cpp 8d1d5c4cb0af911d8dc13e37a1adb62947513d0d
src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 60c7a89fb809582723eb50d22f54f4c8ce697584
Diff: https://reviews.apache.org/r/72442/diff/1/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72442: Reverted the changes about
`REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72442/#review220501
-----------------------------------------------------------
Patch looks great!
Reviews applied: [72442]
Passed command: export OS='ubuntu:16.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/jenkins/buildbot.sh
- Mesos Reviewbot
On April 27, 2020, 2:57 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72442/
> -----------------------------------------------------------
>
> (Updated April 27, 2020, 2:57 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10049
> https://issues.apache.org/jira/browse/MESOS-10049
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When a burstable container is OOM killed because the agent host is running
> out of memory, the method `MemorySubsystemProcess::oomWaited` will NOT be
> invoked, it will only be invoked when the container uses more memory than
> its hard memory limit (i.e., `REASON_CONTAINER_LIMITATION_MEMORY`). So we
> do not need to introduce `REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 9412ed736231547b22abc89188316b08d5445e78
> include/mesos/v1/mesos.proto 194c42cf57e34d803a21cab03db17614855e8692
> src/common/protobuf_utils.cpp 8d1d5c4cb0af911d8dc13e37a1adb62947513d0d
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 60c7a89fb809582723eb50d22f54f4c8ce697584
>
>
> Diff: https://reviews.apache.org/r/72442/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72442: Reverted the changes about
`REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
Posted by Qian Zhang <zh...@gmail.com>.
> On May 5, 2020, 12:27 a.m., Greg Mann wrote:
> > I would recommend updating the description so that instead of saying we "don't need to add" the new reason, say that "it is not possible for Mesos to provide" the reason, so we must remove it.
Done.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72442/#review220599
-----------------------------------------------------------
On May 5, 2020, 3:53 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72442/
> -----------------------------------------------------------
>
> (Updated May 5, 2020, 3:53 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10049
> https://issues.apache.org/jira/browse/MESOS-10049
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The method `MemorySubsystemProcess::oomWaited()` will only be invoked when the
> container is OOM killed because it uses more memory than its hard memory limit
> (i.e., the task status reason `REASON_CONTAINER_LIMITATION_MEMORY`), it will
> NOT be invoked when a burstable container is OOM killed because the agent host
> is running out of memory, i.e., we will NOT receive OOM killing notification
> via cgroups notification API for this case. So it is not possible for Mesos to
> provide a task status reason `REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED` for this
> case.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 9412ed736231547b22abc89188316b08d5445e78
> include/mesos/v1/mesos.proto 194c42cf57e34d803a21cab03db17614855e8692
> src/common/protobuf_utils.cpp 8d1d5c4cb0af911d8dc13e37a1adb62947513d0d
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 60c7a89fb809582723eb50d22f54f4c8ce697584
>
>
> Diff: https://reviews.apache.org/r/72442/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72442: Reverted the changes about
`REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72442/#review220599
-----------------------------------------------------------
Ship it!
I would recommend updating the description so that instead of saying we "don't need to add" the new reason, say that "it is not possible for Mesos to provide" the reason, so we must remove it.
- Greg Mann
On April 27, 2020, 2:57 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72442/
> -----------------------------------------------------------
>
> (Updated April 27, 2020, 2:57 p.m.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10049
> https://issues.apache.org/jira/browse/MESOS-10049
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When a burstable container is OOM killed because the agent host is running
> out of memory, the method `MemorySubsystemProcess::oomWaited` will NOT be
> invoked, it will only be invoked when the container uses more memory than
> its hard memory limit (i.e., `REASON_CONTAINER_LIMITATION_MEMORY`). So we
> do not need to introduce `REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 9412ed736231547b22abc89188316b08d5445e78
> include/mesos/v1/mesos.proto 194c42cf57e34d803a21cab03db17614855e8692
> src/common/protobuf_utils.cpp 8d1d5c4cb0af911d8dc13e37a1adb62947513d0d
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 60c7a89fb809582723eb50d22f54f4c8ce697584
>
>
> Diff: https://reviews.apache.org/r/72442/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 72442: Reverted the changes about
`REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72442/
-----------------------------------------------------------
(Updated May 5, 2020, 3:53 p.m.)
Review request for mesos, Andrei Budnik and Greg Mann.
Changes
-------
Updated description.
Bugs: MESOS-10049
https://issues.apache.org/jira/browse/MESOS-10049
Repository: mesos
Description (updated)
-------
The method `MemorySubsystemProcess::oomWaited()` will only be invoked when the
container is OOM killed because it uses more memory than its hard memory limit
(i.e., the task status reason `REASON_CONTAINER_LIMITATION_MEMORY`), it will
NOT be invoked when a burstable container is OOM killed because the agent host
is running out of memory, i.e., we will NOT receive OOM killing notification
via cgroups notification API for this case. So it is not possible for Mesos to
provide a task status reason `REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED` for this
case.
Diffs (updated)
-----
include/mesos/mesos.proto 9412ed736231547b22abc89188316b08d5445e78
include/mesos/v1/mesos.proto 194c42cf57e34d803a21cab03db17614855e8692
src/common/protobuf_utils.cpp 8d1d5c4cb0af911d8dc13e37a1adb62947513d0d
src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 60c7a89fb809582723eb50d22f54f4c8ce697584
Diff: https://reviews.apache.org/r/72442/diff/2/
Changes: https://reviews.apache.org/r/72442/diff/1-2/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 72442: Reverted the changes about
`REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72442/#review220586
-----------------------------------------------------------
Ship it!
Ship It!
- Andrei Budnik
On Апрель 27, 2020, 2:57 п.п., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72442/
> -----------------------------------------------------------
>
> (Updated Апрель 27, 2020, 2:57 п.п.)
>
>
> Review request for mesos, Andrei Budnik and Greg Mann.
>
>
> Bugs: MESOS-10049
> https://issues.apache.org/jira/browse/MESOS-10049
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When a burstable container is OOM killed because the agent host is running
> out of memory, the method `MemorySubsystemProcess::oomWaited` will NOT be
> invoked, it will only be invoked when the container uses more memory than
> its hard memory limit (i.e., `REASON_CONTAINER_LIMITATION_MEMORY`). So we
> do not need to introduce `REASON_CONTAINER_MEMORY_REQUEST_EXCEEDED`.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 9412ed736231547b22abc89188316b08d5445e78
> include/mesos/v1/mesos.proto 194c42cf57e34d803a21cab03db17614855e8692
> src/common/protobuf_utils.cpp 8d1d5c4cb0af911d8dc13e37a1adb62947513d0d
> src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 60c7a89fb809582723eb50d22f54f4c8ce697584
>
>
> Diff: https://reviews.apache.org/r/72442/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>