You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/11/02 03:36:38 UTC
[GitHub] [bookkeeper] wenbingshen opened a new pull request, #3598: Fix ByteBuf memory leak problem when addEntryInternal
wenbingshen opened a new pull request, #3598:
URL: https://github.com/apache/bookkeeper/pull/3598
### Motivation
The key entry not released after added the key to journal when addEntryInternal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] wenbingshen commented on pull request #3598: Add ByteBuf refCnt test for AddEntry and use pooled ByteBuf reduce heap usage
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on PR #3598:
URL: https://github.com/apache/bookkeeper/pull/3598#issuecomment-1299624862
@StevenLuMT also help take a look. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] merlimat commented on a diff in pull request #3598: Fix ByteBuf memory leak problem when addEntryInternal
Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #3598:
URL: https://github.com/apache/bookkeeper/pull/3598#discussion_r1011127686
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java:
##########
@@ -931,6 +931,18 @@ private Journal getJournal(long ledgerId) {
return journals.get(MathUtils.signSafeMod(ledgerId, journals.size()));
}
+ @VisibleForTesting
+ public ByteBuf createMasterKeyEntry(long ledgerId, byte[] masterKey) {
+ // new handle, we should add the key to journal ensure we can rebuild
+ ByteBuffer bb = ByteBuffer.allocate(8 + 8 + 4 + masterKey.length);
Review Comment:
If we’re returning a ByteBuf and it’s properly ref counted, then we should just use a pooled ByteBuf instance instead of wrapping a ByteBuffer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] wenbingshen commented on a diff in pull request #3598: Add ByteBuf refCnt test for AddEntry and use pooled ByteBuf reduce heap usage
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on code in PR #3598:
URL: https://github.com/apache/bookkeeper/pull/3598#discussion_r1011226656
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java:
##########
@@ -931,6 +931,18 @@ private Journal getJournal(long ledgerId) {
return journals.get(MathUtils.signSafeMod(ledgerId, journals.size()));
}
+ @VisibleForTesting
+ public ByteBuf createMasterKeyEntry(long ledgerId, byte[] masterKey) {
+ // new handle, we should add the key to journal ensure we can rebuild
+ ByteBuffer bb = ByteBuffer.allocate(8 + 8 + 4 + masterKey.length);
Review Comment:
@merlimat Great understanding! Thanks for your review and suggestion. I have used a pooled ByteBuf instead of wrapping a ByteBuffer. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] merlimat merged pull request #3598: Add ByteBuf refCnt test for AddEntry and use pooled ByteBuf reduce heap usage
Posted by GitBox <gi...@apache.org>.
merlimat merged PR #3598:
URL: https://github.com/apache/bookkeeper/pull/3598
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org