Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZOOKEEPER-4199: Avoid thread leak in QuorumRequestPipelineTest #1591

Closed
wants to merge 1 commit into from

Conversation

ztzg
Copy link
Contributor

@ztzg ztzg commented Feb 4, 2021

QuorumRequestPipelineTest hosts parameterized tests which explicitly call QuorumBase.setUp(boolean).

This patch overrides the argument-less QuorumBase.setUp() with an empty body, as the former is annotated @BeforeEach-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of QuorumBase, EagerACLFilterTest.

@ztzg ztzg requested review from eolivelli, nkalmar and symat February 4, 2021 11:24
@ztzg
Copy link
Contributor Author

ztzg commented Feb 4, 2021

Cc: @tamaashu.

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

Copy link
Contributor

@symat symat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! :)
thanks!

`QuorumRequestPipelineTest` hosts parameterized tests which explicitly
calls `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an
empty body, as the former is annotated `@BeforeEach`--otherwise
causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and
immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete,
and while Linux happily handles that load, macOS Catalina's
per-process limit of 2048 threads effectively causes the JVM to
"crash" or lock up.

The solution is copied verbatim from another parameterized subclass of
`QuorumBase`, `EagerACLFilterTest`.
@ztzg ztzg force-pushed the ZOOKEEPER-4199-thread-leak-qrp-test branch from 3d0db80 to 6475ddf Compare February 17, 2021 17:48
@ztzg
Copy link
Contributor Author

ztzg commented Feb 17, 2021

@eolivelli, @symat: Great CI success! Would one of you mind merging this?

@symat
Copy link
Contributor

symat commented Feb 18, 2021

sure, I'll merge soon, thanks @ztzg !!

@asfgit asfgit closed this in 37eae03 Feb 18, 2021
asfgit pushed a commit that referenced this pull request Mar 6, 2021
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes #1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
asfgit pushed a commit that referenced this pull request Mar 6, 2021
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes #1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
@ztzg
Copy link
Contributor Author

ztzg commented Mar 6, 2021

Now also in branch-3.7 and branch-3.7.0. Thanks!

@symat
Copy link
Contributor

symat commented Mar 8, 2021

thanks!

RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Aug 31, 2022
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 3, 2022
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
RokLenarcic pushed a commit to RokLenarcic/zookeeper that referenced this pull request Sep 29, 2022
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
anuragmadnawat1 pushed a commit to anuragmadnawat1/zookeeper that referenced this pull request Nov 2, 2022
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test
anuragmadnawat1 added a commit to anuragmadnawat1/zookeeper that referenced this pull request Nov 2, 2022
`QuorumRequestPipelineTest` hosts parameterized tests which explicitly call `QuorumBase.setUp(boolean)`.

This patch overrides the argument-less `QuorumBase.setUp()` with an empty body, as the former is annotated `BeforeEach`-otherwise causing the runtime to start a fresh 5-ensemble before each test.

Without the override, one such extraneous ensemble is created and immediately leaked for each combination of test method + parameters.

The test consequently requires 4000+ simultaneous threads to complete, and while Linux happily handles that load, macOS Catalina's per-process limit of 2048 threads effectively causes the JVM to "crash" or lock up.

The solution is copied verbatim from another parameterized subclass of `QuorumBase`, `EagerACLFilterTest`.

Author: Damien Diederen <[email protected]>

Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]>

Closes apache#1591 from ztzg/ZOOKEEPER-4199-thread-leak-qrp-test

Co-authored-by: Damien Diederen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants