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][broker] Fix thread safety of loadSheddingTask and loadResourceQuotaTask fields #22660

Merged
merged 1 commit into from
May 7, 2024
Merged

[fix][broker] Fix thread safety of loadSheddingTask and loadResourceQuotaTask fields #22660

merged 1 commit into from
May 7, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented May 6, 2024

Motivation

When investigating OOM issues in tests, I noticed that some memory leaks were related to references via LoadSheddingTask
image

There are 2 problems:

  • the fields aren't accessed in a thread safety way
  • because of races, it's possible that the task gets started when the instance is already shutting down.

Modifications

  • extract methods for cancelling and creating the loadSheddingTask and loadResourceQuotaTask instances
  • make the methods synchronized to fix the thread safety issue
  • before creating the instances, check that PulsarService is running

Documentation

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

@lhotari lhotari added this to the 3.3.0 milestone May 6, 2024
@lhotari lhotari requested review from merlimat and heesung-sn May 6, 2024 18:31
@lhotari lhotari self-assigned this May 6, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label May 6, 2024
@merlimat merlimat merged commit 1e19190 into apache:master May 7, 2024
49 of 53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@heesung-sn heesung-sn heesung-sn approved these changes

@dragosvictor dragosvictor dragosvictor approved these changes

@merlimat merlimat Awaiting requested review from merlimat

Assignees

@lhotari lhotari

Labels
doc-not-needed Your PR changes do not impact docs ready-to-test
Projects
None yet
Milestone
3.3.0
Development

Successfully merging this pull request may close these issues.
4 participants
@lhotari @dragosvictor @heesung-sn @merlimat