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 2018/10/26 03:26:43 UTC

Review Request 69188: Fixed a wrong way to install callback for OOM notifier.

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
-------

Fixed a wrong way to install callback for OOM notifier.


Diffs
-----

  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp f4ed8337253995250cb533fa0a232909cb1a824b 


Diff: https://reviews.apache.org/r/69188/diff/1/


Testing
-------


Thanks,

Qian Zhang


Re: Review Request 69188: Ensured failed / discarded cgroups OOM notification is logged.

Posted by Qian Zhang <zh...@gmail.com>.

> On Oct. 27, 2018, 2:52 a.m., Benjamin Mahler wrote:
> > Perhaps a little more description would be helpful? E.g.
> > 
> > ```
> > Ensured failed / discarded cgroup oom notification is logged.
> > 
> > Failed or discarded oom notificaitions in the cgroup memory
> > isolator were not being logged, due to the continuation being
> > accidentally set up using onReady rather than onAny.
> > ```
> > 
> > This tells the user what functionally is changing (they will now see logging if it fails or is discarded).

Agree!


- Qian


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


On Oct. 29, 2018, 12:11 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69188/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2018, 12:11 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9334
>     https://issues.apache.org/jira/browse/MESOS-9334
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Failed or discarded OOM notificaitions in the cgroups memory
> subsystem were not being logged, due to the continuation being
> accidentally set up using `onReady` rather than `onAny`.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp f4ed8337253995250cb533fa0a232909cb1a824b 
> 
> 
> Diff: https://reviews.apache.org/r/69188/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 69188: Fixed a wrong way to install callback for OOM notifier.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69188/#review210104
-----------------------------------------------------------


Ship it!




Perhaps a little more description would be helpful? E.g.

```
Ensured failed / discarded cgroup oom notification is logged.

Failed or discarded oom notificaitions in the cgroup memory
isolator were not being logged, due to the continuation being
accidentally set up using onReady rather than onAny.
```

This tells the user what functionally is changing (they will now see logging if it fails or is discarded).

- Benjamin Mahler


On Oct. 26, 2018, 3:26 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69188/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2018, 3:26 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9334
>     https://issues.apache.org/jira/browse/MESOS-9334
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fixed a wrong way to install callback for OOM notifier.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp f4ed8337253995250cb533fa0a232909cb1a824b 
> 
> 
> Diff: https://reviews.apache.org/r/69188/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 69188: Fixed a wrong way to install callback for OOM notifier.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69188/#review210077
-----------------------------------------------------------



PASS: Mesos patch 69188 was successfully built and tested.

Reviews applied: `['69123', '69188']`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2536/mesos-review-69188

- Mesos Reviewbot Windows


On Oct. 26, 2018, 3:26 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69188/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2018, 3:26 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9334
>     https://issues.apache.org/jira/browse/MESOS-9334
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Fixed a wrong way to install callback for OOM notifier.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp f4ed8337253995250cb533fa0a232909cb1a824b 
> 
> 
> Diff: https://reviews.apache.org/r/69188/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 69188: Ensured failed / discarded cgroups OOM notification is logged.

Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69188/#review210180
-----------------------------------------------------------


Ship it!




Ship It!

- Gilbert Song


On Oct. 28, 2018, 9:11 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69188/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2018, 9:11 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9334
>     https://issues.apache.org/jira/browse/MESOS-9334
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Failed or discarded OOM notificaitions in the cgroups memory
> subsystem were not being logged, due to the continuation being
> accidentally set up using `onReady` rather than `onAny`.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp f4ed8337253995250cb533fa0a232909cb1a824b 
> 
> 
> Diff: https://reviews.apache.org/r/69188/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 69188: Ensured failed / discarded cgroups OOM notification is logged.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69188/#review210137
-----------------------------------------------------------



Patch looks great!

Reviews applied: [69123, 69188]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Oct. 29, 2018, 4:11 a.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69188/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2018, 4:11 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9334
>     https://issues.apache.org/jira/browse/MESOS-9334
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Failed or discarded OOM notificaitions in the cgroups memory
> subsystem were not being logged, due to the continuation being
> accidentally set up using `onReady` rather than `onAny`.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp f4ed8337253995250cb533fa0a232909cb1a824b 
> 
> 
> Diff: https://reviews.apache.org/r/69188/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 69188: Ensured failed / discarded cgroups OOM notification is logged.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69188/#review210135
-----------------------------------------------------------



PASS: Mesos patch 69188 was successfully built and tested.

Reviews applied: `['69123', '69188']`

All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2543/mesos-review-69188

- Mesos Reviewbot Windows


On Oct. 28, 2018, 9:11 p.m., Qian Zhang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69188/
> -----------------------------------------------------------
> 
> (Updated Oct. 28, 2018, 9:11 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9334
>     https://issues.apache.org/jira/browse/MESOS-9334
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Failed or discarded OOM notificaitions in the cgroups memory
> subsystem were not being logged, due to the continuation being
> accidentally set up using `onReady` rather than `onAny`.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp f4ed8337253995250cb533fa0a232909cb1a824b 
> 
> 
> Diff: https://reviews.apache.org/r/69188/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>


Re: Review Request 69188: Ensured failed / discarded cgroups OOM notification is logged.

Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69188/
-----------------------------------------------------------

(Updated Oct. 29, 2018, 12:11 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
-------

Updated commit message.


Summary (updated)
-----------------

Ensured failed / discarded cgroups OOM notification is logged.


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


Repository: mesos


Description (updated)
-------

Failed or discarded OOM notificaitions in the cgroups memory
subsystem were not being logged, due to the continuation being
accidentally set up using `onReady` rather than `onAny`.


Diffs (updated)
-----

  src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp f4ed8337253995250cb533fa0a232909cb1a824b 


Diff: https://reviews.apache.org/r/69188/diff/2/

Changes: https://reviews.apache.org/r/69188/diff/1-2/


Testing
-------


Thanks,

Qian Zhang