You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Dmitry Zhuk <dz...@twopensource.com> on 2017/10/13 10:47:11 UTC
Re: Review Request 60467: Fixed race in configureSignal and
installCtrlHandler.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60467/
-----------------------------------------------------------
(Updated Oct. 13, 2017, 10:47 a.m.)
Review request for mesos, Alexander Rukletsov, Anand Mazumdar, and Avinash sridharan.
Summary (updated)
-----------------
Fixed race in configureSignal and installCtrlHandler.
Bugs: MESOS-8084
https://issues.apache.org/jira/browse/MESOS-8084
Repository: mesos
Description (updated)
-------
When multiple agents are spawned in a single process, they attempt to
install signal handler concurrently, causing tests, such as
`ExamplesTest.PersistentVolumeFramework` and
`ExamplesTest.DynamicReservationFramework` randomly fail. This adds
missing synchronization to `configureSignal` and `installCtrlHandler`
to resolve the issue.
Diffs (updated)
-----
src/slave/posix_signalhandler.hpp 49692b140662280b9e150654c5f336f1b920586f
src/slave/windows_ctrlhandler.hpp 145a232bcc8428224ac235864845ab91615ca587
Diff: https://reviews.apache.org/r/60467/diff/2/
Changes: https://reviews.apache.org/r/60467/diff/1-2/
Testing (updated)
-------
On Linux:
```
make check
while true; do ./mesos-tests.sh --gtest_filter="ExamplesTest.DynamicReservationFramework:ExamplesTest.PersistentVolumeFramework"; done
```
Had several crashes before the fix and none after.
Don't have access to Windows machine, so can't compile ant test.
Thanks,
Dmitry Zhuk
Re: Review Request 60467: Fixed race in configureSignal and
installCtrlHandler.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60467/#review188061
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/posix_signalhandler.hpp
Lines 23-24 (original), 28-29 (patched)
<https://reviews.apache.org/r/60467/#comment265137>
Since we have synchronization now, the last sentence is not true any more?
src/slave/windows_ctrlhandler.hpp
Lines 25-26 (original), 28-29 (patched)
<https://reviews.apache.org/r/60467/#comment265138>
Ditto
- Alexander Rukletsov
On Oct. 13, 2017, 10:47 a.m., Dmitry Zhuk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60467/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2017, 10:47 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, and Avinash sridharan.
>
>
> Bugs: MESOS-8084
> https://issues.apache.org/jira/browse/MESOS-8084
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When multiple agents are spawned in a single process, they attempt to
> install signal handler concurrently, causing tests, such as
> `ExamplesTest.PersistentVolumeFramework` and
> `ExamplesTest.DynamicReservationFramework` randomly fail. This adds
> missing synchronization to `configureSignal` and `installCtrlHandler`
> to resolve the issue.
>
>
> Diffs
> -----
>
> src/slave/posix_signalhandler.hpp 49692b140662280b9e150654c5f336f1b920586f
> src/slave/windows_ctrlhandler.hpp 145a232bcc8428224ac235864845ab91615ca587
>
>
> Diff: https://reviews.apache.org/r/60467/diff/2/
>
>
> Testing
> -------
>
> On Linux:
> ```
> make check
> while true; do ./mesos-tests.sh --gtest_filter="ExamplesTest.DynamicReservationFramework:ExamplesTest.PersistentVolumeFramework"; done
> ```
> Had several crashes before the fix and none after.
>
> Don't have access to Windows machine, so can't compile ant test.
>
>
> Thanks,
>
> Dmitry Zhuk
>
>
Re: Review Request 60467: Fixed race in configureSignal and
installCtrlHandler.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60467/#review187956
-----------------------------------------------------------
PASS: Mesos patch 60467 was successfully built and tested.
Reviews applied: `['60467']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60467
- Mesos Reviewbot Windows
On Oct. 13, 2017, 10:47 a.m., Dmitry Zhuk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60467/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2017, 10:47 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, and Avinash sridharan.
>
>
> Bugs: MESOS-8084
> https://issues.apache.org/jira/browse/MESOS-8084
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When multiple agents are spawned in a single process, they attempt to
> install signal handler concurrently, causing tests, such as
> `ExamplesTest.PersistentVolumeFramework` and
> `ExamplesTest.DynamicReservationFramework` randomly fail. This adds
> missing synchronization to `configureSignal` and `installCtrlHandler`
> to resolve the issue.
>
>
> Diffs
> -----
>
> src/slave/posix_signalhandler.hpp 49692b140662280b9e150654c5f336f1b920586f
> src/slave/windows_ctrlhandler.hpp 145a232bcc8428224ac235864845ab91615ca587
>
>
> Diff: https://reviews.apache.org/r/60467/diff/2/
>
>
> Testing
> -------
>
> On Linux:
> ```
> make check
> while true; do ./mesos-tests.sh --gtest_filter="ExamplesTest.DynamicReservationFramework:ExamplesTest.PersistentVolumeFramework"; done
> ```
> Had several crashes before the fix and none after.
>
> Don't have access to Windows machine, so can't compile ant test.
>
>
> Thanks,
>
> Dmitry Zhuk
>
>
Re: Review Request 60467: Fixed race in configureSignal and
installCtrlHandler.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60467/#review187952
-----------------------------------------------------------
Patch looks great!
Reviews applied: [60467]
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. 13, 2017, 10:47 a.m., Dmitry Zhuk wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60467/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2017, 10:47 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, and Avinash sridharan.
>
>
> Bugs: MESOS-8084
> https://issues.apache.org/jira/browse/MESOS-8084
>
>
> Repository: mesos
>
>
> Description
> -------
>
> When multiple agents are spawned in a single process, they attempt to
> install signal handler concurrently, causing tests, such as
> `ExamplesTest.PersistentVolumeFramework` and
> `ExamplesTest.DynamicReservationFramework` randomly fail. This adds
> missing synchronization to `configureSignal` and `installCtrlHandler`
> to resolve the issue.
>
>
> Diffs
> -----
>
> src/slave/posix_signalhandler.hpp 49692b140662280b9e150654c5f336f1b920586f
> src/slave/windows_ctrlhandler.hpp 145a232bcc8428224ac235864845ab91615ca587
>
>
> Diff: https://reviews.apache.org/r/60467/diff/2/
>
>
> Testing
> -------
>
> On Linux:
> ```
> make check
> while true; do ./mesos-tests.sh --gtest_filter="ExamplesTest.DynamicReservationFramework:ExamplesTest.PersistentVolumeFramework"; done
> ```
> Had several crashes before the fix and none after.
>
> Don't have access to Windows machine, so can't compile ant test.
>
>
> Thanks,
>
> Dmitry Zhuk
>
>