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 2023/01/17 15:41:11 UTC
[GitHub] [bookkeeper] horizonzy opened a new pull request, #3746: Fix memory leak when read only
horizonzy opened a new pull request, #3746:
URL: https://github.com/apache/bookkeeper/pull/3746
Descriptions of the changes in this PR:
Fixes #3745
--
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] horizonzy commented on a diff in pull request #3746: Fix memory leak when read only
Posted by GitBox <gi...@apache.org>.
horizonzy commented on code in PR #3746:
URL: https://github.com/apache/bookkeeper/pull/3746#discussion_r1073111538
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java:
##########
@@ -175,7 +175,9 @@ public void run() {
if (request instanceof BookieProtocol.ReadRequest) {
requestProcessor.onReadRequestFinish();
}
- if (request instanceof BookieProtocol.AddRequest) {
+ if (request instanceof BookieProtocol.ParsedAddRequest) {
+ ((BookieProtocol.ParsedAddRequest) request).release();
+ request.recycle();
requestProcessor.onAddRequestFinish();
Review Comment:
It can't, we shouldn't release the data in the recycle.
In the normal case, it will recycle the request, but the data is already free.
See: https://github.com/apache/bookkeeper/blob/7b5b6b240ca7551da266903078f2dd2ba2906b96/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/WriteEntryProcessor.java#L131
--
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 #3746: Fix memory leak when read only
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on code in PR #3746:
URL: https://github.com/apache/bookkeeper/pull/3746#discussion_r1073074566
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java:
##########
@@ -175,7 +175,9 @@ public void run() {
if (request instanceof BookieProtocol.ReadRequest) {
requestProcessor.onReadRequestFinish();
}
- if (request instanceof BookieProtocol.AddRequest) {
+ if (request instanceof BookieProtocol.ParsedAddRequest) {
+ ((BookieProtocol.ParsedAddRequest) request).release();
+ request.recycle();
requestProcessor.onAddRequestFinish();
Review Comment:
Can the release of ParsedAddRequest be executed in the recycle method? It seems that these two methods are always used together, and there is a forced switch here, which seems a bit awkward.
You can look at org.apache.bookkeeper.proto.BookieProtocol.AddRequest#recycle method
```java
@Override
public void recycle() {
ledgerId = -1;
entryId = -1;
masterKey = null;
ReferenceCountUtil.safeRelease(data);
data = null;
recyclerHandle.recycle(this);
}
```
--
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] hangc0276 merged pull request #3746: [bookie] Fix memory leak when the Bookie is in read only mode
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 merged PR #3746:
URL: https://github.com/apache/bookkeeper/pull/3746
--
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 #3746: Fix memory leak when read only
Posted by GitBox <gi...@apache.org>.
wenbingshen commented on code in PR #3746:
URL: https://github.com/apache/bookkeeper/pull/3746#discussion_r1073135434
##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java:
##########
@@ -175,7 +175,9 @@ public void run() {
if (request instanceof BookieProtocol.ReadRequest) {
requestProcessor.onReadRequestFinish();
}
- if (request instanceof BookieProtocol.AddRequest) {
+ if (request instanceof BookieProtocol.ParsedAddRequest) {
+ ((BookieProtocol.ParsedAddRequest) request).release();
+ request.recycle();
requestProcessor.onAddRequestFinish();
Review Comment:
I saw it, sorry I misread it earlier. Nice work. 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] hangc0276 commented on pull request #3746: Fix memory leak when read only
Posted by "hangc0276 (via GitHub)" <gi...@apache.org>.
hangc0276 commented on PR #3746:
URL: https://github.com/apache/bookkeeper/pull/3746#issuecomment-1414708987
@merlimat @eolivelli @dlg99 @zymap Please help take a look at this PR, 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