You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rojas <al...@mesosphere.io> on 2016/02/23 20:08:53 UTC
Re: Review Request 43799: WIP: Removed race condition from libevent
based poll implementation.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/
-----------------------------------------------------------
(Updated Feb. 23, 2016, 8:08 p.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Changes
-------
Marking as WIP, there are still issues here.
Summary (updated)
-----------------
WIP: Removed race condition from libevent based poll implementation.
Bugs: MESOS-3271 and MESOS-4711
https://issues.apache.org/jira/browse/MESOS-3271
https://issues.apache.org/jira/browse/MESOS-4711
Repository: mesos
Description (updated)
-------
WIP: Patch not ready for commit.
Under certains circumstances, the future returned by poll is discarded right
after the event is triggered, this causes the event callback to be called
before the discard callback which results in an abort signal being raised
by the libevent library.
Diffs
-----
3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
Diff: https://reviews.apache.org/r/43799/diff/
Testing
-------
```bash
# On CentOS 6.7 running in virtualbox
../configure --enable-ssl --enable-libevent
make -j4 check
sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
```
Thanks,
Alexander Rojas
Re: Review Request 43799: WIP: Removed race condition from libevent
based poll implementation.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/#review120416
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43799]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 23, 2016, 7:08 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43799/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2016, 7:08 p.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-3271 and MESOS-4711
> https://issues.apache.org/jira/browse/MESOS-3271
> https://issues.apache.org/jira/browse/MESOS-4711
>
>
> Repository: mesos
>
>
> Description
> -------
>
> WIP: Patch not ready for commit.
>
> Under certains circumstances, the future returned by poll is discarded right
> after the event is triggered, this causes the event callback to be called
> before the discard callback which results in an abort signal being raised
> by the libevent library.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
>
> Diff: https://reviews.apache.org/r/43799/diff/
>
>
> Testing
> -------
>
> ```bash
> # On CentOS 6.7 running in virtualbox
> ../configure --enable-ssl --enable-libevent
> make -j4 check
> sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 43799: Removed race condition from libevent based
poll implementation.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/#review120932
-----------------------------------------------------------
3rdparty/libprocess/src/libevent_poll.cpp (line 50)
<https://reviews.apache.org/r/43799/#comment182470>
Please add a very clear comment here that calls out the effect of this destructor invoking the event_free, which implies `event_del` which means the event will no longer be `pending`, and that's why we won't execute a 2nd time
3rdparty/libprocess/src/libevent_poll.cpp (lines 61 - 63)
<https://reviews.apache.org/r/43799/#comment182468>
can you add a comment explaining why we are testing `event_pending`?
Another comment for explaining that `event_active` triggers pollCallback to be run in the `event_loop`
3rdparty/libprocess/src/libevent_poll.cpp (lines 83 - 85)
<https://reviews.apache.org/r/43799/#comment182467>
let's call out here that we're binding in the destructor, and the consequences of that.
3rdparty/libprocess/src/libevent_poll.cpp (lines 90 - 91)
<https://reviews.apache.org/r/43799/#comment182461>
Please comment why we're using a weak_ptr here.
3rdparty/libprocess/src/libevent_poll.cpp (lines 91 - 93)
<https://reviews.apache.org/r/43799/#comment182462>
Please comment that this is the explicit order we want these 2 statements in.
- Joris Van Remoortere
On Feb. 24, 2016, 9:30 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43799/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 9:30 p.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-3271 and MESOS-4711
> https://issues.apache.org/jira/browse/MESOS-3271
> https://issues.apache.org/jira/browse/MESOS-4711
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Under certains circumstances, the future returned by poll is discarded right
> after the event is triggered, this causes the event callback to be called
> before the discard callback which results in an abort signal being raised
> by the libevent library.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
>
> Diff: https://reviews.apache.org/r/43799/diff/
>
>
> Testing
> -------
>
> ```bash
> # On CentOS 6.7 running in virtualbox
> ../configure --enable-ssl --enable-libevent
> make -j4 check
> sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 43799: Removed race condition from libevent based
poll implementation.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/
-----------------------------------------------------------
(Updated Feb. 27, 2016, 1:24 a.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Changes
-------
Fixed IO tests.
Bugs: MESOS-3271 and MESOS-4711
https://issues.apache.org/jira/browse/MESOS-3271
https://issues.apache.org/jira/browse/MESOS-4711
Repository: mesos
Description
-------
Under certains circumstances, the future returned by poll is discarded right
after the event is triggered, this causes the event callback to be called
before the discard callback which results in an abort signal being raised
by the libevent library.
Diffs (updated)
-----
3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
Diff: https://reviews.apache.org/r/43799/diff/
Testing (updated)
-------
```bash
# On CentOS 6.7 running in virtualbox
../configure --enable-ssl --enable-libevent
make -j4 check
./3rdpaty/libprocess/libprocess-tests --gtest_repeat=200 --gtest_break_on_failure
sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
```
Thanks,
Alexander Rojas
Re: Review Request 43799: Removed race condition from libevent based
poll implementation.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/#review120998
-----------------------------------------------------------
Ship it!
Ship It!
- Joris Van Remoortere
On Feb. 26, 2016, 9:38 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43799/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2016, 9:38 p.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-3271 and MESOS-4711
> https://issues.apache.org/jira/browse/MESOS-3271
> https://issues.apache.org/jira/browse/MESOS-4711
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Under certains circumstances, the future returned by poll is discarded right
> after the event is triggered, this causes the event callback to be called
> before the discard callback which results in an abort signal being raised
> by the libevent library.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
>
> Diff: https://reviews.apache.org/r/43799/diff/
>
>
> Testing
> -------
>
> ```bash
> # On CentOS 6.7 running in virtualbox
> ../configure --enable-ssl --enable-libevent
> make -j4 check
> sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 43799: Removed race condition from libevent based
poll implementation.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/#review121002
-----------------------------------------------------------
This breaks the IO based tests in Libprocess.
- Joris Van Remoortere
On Feb. 26, 2016, 9:38 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43799/
> -----------------------------------------------------------
>
> (Updated Feb. 26, 2016, 9:38 p.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-3271 and MESOS-4711
> https://issues.apache.org/jira/browse/MESOS-3271
> https://issues.apache.org/jira/browse/MESOS-4711
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Under certains circumstances, the future returned by poll is discarded right
> after the event is triggered, this causes the event callback to be called
> before the discard callback which results in an abort signal being raised
> by the libevent library.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
>
> Diff: https://reviews.apache.org/r/43799/diff/
>
>
> Testing
> -------
>
> ```bash
> # On CentOS 6.7 running in virtualbox
> ../configure --enable-ssl --enable-libevent
> make -j4 check
> sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 43799: Removed race condition from libevent based
poll implementation.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/
-----------------------------------------------------------
(Updated Feb. 26, 2016, 10:38 p.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Changes
-------
Fixed comment formatting.
Bugs: MESOS-3271 and MESOS-4711
https://issues.apache.org/jira/browse/MESOS-3271
https://issues.apache.org/jira/browse/MESOS-4711
Repository: mesos
Description
-------
Under certains circumstances, the future returned by poll is discarded right
after the event is triggered, this causes the event callback to be called
before the discard callback which results in an abort signal being raised
by the libevent library.
Diffs (updated)
-----
3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
Diff: https://reviews.apache.org/r/43799/diff/
Testing
-------
```bash
# On CentOS 6.7 running in virtualbox
../configure --enable-ssl --enable-libevent
make -j4 check
sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
```
Thanks,
Alexander Rojas
Re: Review Request 43799: Removed race condition from libevent based
poll implementation.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/
-----------------------------------------------------------
(Updated Feb. 26, 2016, 10:29 p.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Changes
-------
Added comments.
Bugs: MESOS-3271 and MESOS-4711
https://issues.apache.org/jira/browse/MESOS-3271
https://issues.apache.org/jira/browse/MESOS-4711
Repository: mesos
Description
-------
Under certains circumstances, the future returned by poll is discarded right
after the event is triggered, this causes the event callback to be called
before the discard callback which results in an abort signal being raised
by the libevent library.
Diffs (updated)
-----
3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
Diff: https://reviews.apache.org/r/43799/diff/
Testing
-------
```bash
# On CentOS 6.7 running in virtualbox
../configure --enable-ssl --enable-libevent
make -j4 check
sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
```
Thanks,
Alexander Rojas
Re: Review Request 43799: Removed race condition from libevent based
poll implementation.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/#review120680
-----------------------------------------------------------
Patch looks great!
Reviews applied: [43799]
Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh
- Mesos ReviewBot
On Feb. 24, 2016, 9:30 p.m., Alexander Rojas wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43799/
> -----------------------------------------------------------
>
> (Updated Feb. 24, 2016, 9:30 p.m.)
>
>
> Review request for mesos, Joris Van Remoortere and Michael Park.
>
>
> Bugs: MESOS-3271 and MESOS-4711
> https://issues.apache.org/jira/browse/MESOS-3271
> https://issues.apache.org/jira/browse/MESOS-4711
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Under certains circumstances, the future returned by poll is discarded right
> after the event is triggered, this causes the event callback to be called
> before the discard callback which results in an abort signal being raised
> by the libevent library.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
>
> Diff: https://reviews.apache.org/r/43799/diff/
>
>
> Testing
> -------
>
> ```bash
> # On CentOS 6.7 running in virtualbox
> ../configure --enable-ssl --enable-libevent
> make -j4 check
> sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
> ```
>
>
> Thanks,
>
> Alexander Rojas
>
>
Re: Review Request 43799: Removed race condition from libevent based
poll implementation.
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43799/
-----------------------------------------------------------
(Updated Feb. 24, 2016, 10:30 p.m.)
Review request for mesos, Joris Van Remoortere and Michael Park.
Summary (updated)
-----------------
Removed race condition from libevent based poll implementation.
Bugs: MESOS-3271 and MESOS-4711
https://issues.apache.org/jira/browse/MESOS-3271
https://issues.apache.org/jira/browse/MESOS-4711
Repository: mesos
Description (updated)
-------
Under certains circumstances, the future returned by poll is discarded right
after the event is triggered, this causes the event callback to be called
before the discard callback which results in an abort signal being raised
by the libevent library.
Diffs (updated)
-----
3rdparty/libprocess/src/libevent_poll.cpp 461624ca003e97be5ea9cf956d86fc294e6f1bc1
Diff: https://reviews.apache.org/r/43799/diff/
Testing
-------
```bash
# On CentOS 6.7 running in virtualbox
../configure --enable-ssl --enable-libevent
make -j4 check
sudo ./bin/mesos-tests.sh --gtest_filter="MemoryPressureMesosTest.CGROUPS_ROOT_SlaveRecovery" --gtest_repeat=1000
```
Thanks,
Alexander Rojas