fix memory leak #9194
fix memory leak #9194
Conversation
|
/pulsarbot run-failure-checks |
codelipenghui
requested review from
sijie,
315157973,
hangc0276,
jiazhai and
merlimat
| @@ -99,6 +100,12 @@ public void onManagedLedgerLastLedgerInitialize(String name, LedgerHandle lh) { | |||
| } | |||
| } catch (org.apache.bookkeeper.client.api.BKException | InterruptedException e) { | |||
| log.error("[{}] Read last entry error.", name, e); | |||
| } finally { | |||
| if (ledgerEntries != null) { | |||
There was a problem hiding this comment.
LedgerEntries has a close() interface.
|
/pulsarbot run-failure-checks |
| try { | ||
| for (BrokerEntryMetadataInterceptor interceptor : brokerEntryMetadataInterceptors) { | ||
| if (interceptor instanceof AppendIndexMetadataInterceptor && lh.getLastAddConfirmed() >= 0) { | ||
| LedgerEntries ledgerEntries = | ||
| ledgerEntries = |
There was a problem hiding this comment.
What happens if we get to here multiple times?
There is no invariant that enforces the presence of only one AppendIndexMetadataInterceptor
There was a problem hiding this comment.
You can use a try-with-resources block and close the object as soon as we do not it anymore
There was a problem hiding this comment.
There will be only one AppendIndexMetadataInterceptor for each ManagedLedger, it was set before open a new ManagedLedger
There was a problem hiding this comment.
what about
try (LedgerEntries ledgerEntries = lh.read....) {
...
}
There was a problem hiding this comment.
try-with-resources is alternative way and makes code more tidy.
I think it's better to raise an individual issue for refactor code like try-with-resources
|
/pulsarbot run-failure-checks |
Motivation
This pr fix memory leak by release bytebuf.