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.

Sign up for GitHub

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

Jump to bottom

[fix][test] Fix thread leaks in Managed Ledger tests and remove duplicate shutdown code #21426

Merged
merged 2 commits into from
Oct 24, 2023
Merged

[fix][test] Fix thread leaks in Managed Ledger tests and remove duplicate shutdown code #21426

merged 2 commits into from
Oct 24, 2023

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Oct 23, 2023

Motivation

Managed Ledger tests leak threads.

Modifications

  • remove duplicate shutdown code in ManagedLedgerFactoryImpl
    • shutdownAsync and shutdown contained almost identical code. Make shutdown use shutdownAsync instead of duplicating the code
  • Fix an issue in the async shutdown code where the executor couldn't be closed since it was called from one of the executor threads
  • fix issue in MockedBookKeeperTestCase cleanup where the metadata store didn't get closed when the factory had already been shutdown in the test
  • add diagnostics to metadata store threadpool to identify which test created the metadata store
  • metadata store threads didn't have an identifier when the metadata store name was empty. Fix this issue.
  • the changes in metadata store naming will help identify the threads in the future

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@lhotari lhotari added this to the 3.2.0 milestone Oct 23, 2023
@lhotari lhotari self-assigned this Oct 23, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Oct 23, 2023
@lhotari
Copy link
Member Author

lhotari commented Oct 23, 2023

/pulsarbot rerun-failure-checks

@lhotari lhotari marked this pull request as draft October 24, 2023 01:58
@lhotari lhotari marked this pull request as ready for review October 24, 2023 05:19
lhotari added a commit to lhotari/pulsar that referenced this pull request Oct 24, 2023
@lhotari
[fix][test] Fix thread leaks in Managed Ledger tests (apache#21426)
e1a7097
@lhotari lhotari merged commit c6704df into apache:master Oct 24, 2023
51 checks passed
lhotari added a commit that referenced this pull request Jun 19, 2024
@lhotari
[fix][test] Fix thread leaks in Managed Ledger tests and remove dupli…
eb9a95d 
…cate shutdown code (#21426)

(cherry picked from commit c6704df)
@lhotari lhotari mentioned this pull request Jun 19, 2024
Merged
4 tasks
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 21, 2024
@lhotari @nikhil-ctds
[fix][test] Fix thread leaks in Managed Ledger tests and remove dupli…
121781c 
…cate shutdown code (apache#21426)

(cherry picked from commit c6704df)
(cherry picked from commit eb9a95d)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 24, 2024
@lhotari @nikhil-ctds
[fix][test] Fix thread leaks in Managed Ledger tests and remove dupli…
5775898 
…cate shutdown code (apache#21426)

(cherry picked from commit c6704df)
(cherry picked from commit eb9a95d)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jun 25, 2024
@lhotari @nikhil-ctds
[fix][test] Fix thread leaks in Managed Ledger tests and remove dupli…
8aafc9d 
…cate shutdown code (apache#21426)

(cherry picked from commit c6704df)
(cherry picked from commit eb9a95d)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 1, 2024
@lhotari @srinath-ctds
[fix][test] Fix thread leaks in Managed Ledger tests and remove dupli…
2c987c8 
…cate shutdown code (apache#21426)

(cherry picked from commit c6704df)
(cherry picked from commit eb9a95d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Technoboy- Technoboy- Technoboy- approved these changes

@Jason918 Jason918 Awaiting requested review from Jason918

@tisonkun tisonkun Awaiting requested review from tisonkun

@poorbarcode poorbarcode Awaiting requested review from poorbarcode

@mattisonchao mattisonchao Awaiting requested review from mattisonchao

Assignees

@lhotari lhotari

Labels
cherry-picked/branch-3.0 doc-not-needed Your PR changes do not impact docs ready-to-test release/3.0.6 type/flaky-tests
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@lhotari @Technoboy-