-
Notifications
You must be signed in to change notification settings - Fork 896
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
[fix][server]fix memory leak when operating ledger metadata #3662
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
Would you please add a test to protect this logic? thanks.
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java
Outdated
Show resolved
Hide resolved
bookkeeper-server/src/main/java/org/apache/bookkeeper/meta/CleanupLedgerManager.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
added a unit test. PTAL @hangc0276 |
* [fix][server]fix memory leak when operating ledger metadata (cherry picked from commit 109688c)
* [fix][server]fix memory leak when operating ledger metadata (cherry picked from commit 109688c)
Motivation
There is a memory leak point in Bookkeeper client, see the following heap report:
![image](https://user-images.githubusercontent.com/4970972/203212033-09337cf0-afd1-4b0c-9ddf-3ba246e85849.png)
It happens when opening a non-existent or expired ledger data. The following screen shot is memeory leak point by Eclipse Memory Analyzer.
![image](https://user-images.githubusercontent.com/4970972/203212114-d75cfcbc-f8d1-4880-977f-aa1847acb837.png)
Changes
Handle exception and clean up when promise complete exceptionally.
Master Issue: #1