You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by "slfan1989 (via GitHub)" <gi...@apache.org> on 2026/02/22 03:35:39 UTC
[PR] RATIS-2417. Fix ChannelMap memory leak in DataStreamManagement when read() fails early. [ratis]
slfan1989 opened a new pull request, #1358:
URL: https://github.com/apache/ratis/pull/1358
## What changes were proposed in this pull request?
Fixed a memory leak in `DataStreamManagement` where `ChannelMap` entries were not properly cleaned up when `read()` fails early (e.g., protocol parsing errors before `readImpl()` completes).
#### Changes:
- Extracted `ClientInvocationId` and `ChannelId` calculation before calling `readImpl()` to ensure they are available in the exception handler
- Added `channels.remove(channelId, key)` in the `read()` exception handler to properly clean up `ChannelMap` entries
## What is the link to the Apache JIRA
JIRA: RATIS-2417. Fix ChannelMap memory leak in DataStreamManagement when read() fails early.
## How was this patch tested?
Added `TestDataStreamManagement` unit test in ratis-test module.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2417. Fix ChannelMap memory leak in DataStreamManagement when read() fails early. [ratis]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #1358:
URL: https://github.com/apache/ratis/pull/1358#issuecomment-3954623497
> > ... The test `TestRaftAsyncWithNetty.testWithLoadAsync` often times out when running on the server/CI environment. After checking it locally, the only thing I suspect is causing the issue is that this test generates a very large volume of logs. ...
>
> Thanks for pointing it out. Just have downloaded [a test log](https://github.com/apache/ratis/actions/runs/22269726843/attempts/3?pr=1358) and found that TestRaftAsyncWithNetty is printing 1.6m lines now.
>
> ```java
> // NettyRpcProxy.Connection
> p.addLast(new LoggingHandler(LogLevel.WARN));
> ```
>
> I probably have misunderstood that meaning of passing `LogLevel.WARN` above -- I thought it would print only the WARN messages but it actually printed all the log messages using WARN. Filed [RATIS-2418](https://issues.apache.org/jira/browse/RATIS-2418).
@szetszwo Thank you very much for helping review the code!
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2417. Fix ChannelMap memory leak in DataStreamManagement when read() fails early. [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #1358:
URL: https://github.com/apache/ratis/pull/1358#issuecomment-3953892635
> ... The test `TestRaftAsyncWithNetty.testWithLoadAsync` often times out when running on the server/CI environment. After checking it locally, the only thing I suspect is causing the issue is that this test generates a very large volume of logs. ...
Thanks for pointing it out. Just have downloaded [a test log](https://github.com/apache/ratis/actions/runs/22269726843/attempts/3?pr=1358) and found that TestRaftAsyncWithNetty is printing 1.6m lines now.
```java
// NettyRpcProxy.Connection
p.addLast(new LoggingHandler(LogLevel.WARN));
```
I probably have misunderstood that meaning of passing `LogLevel.WARN` above -- I thought it would print only the WARN messages but it actually printed all the log messages using WARN. Filed RATIS-2418.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2417. Fix ChannelMap memory leak in DataStreamManagement when read() fails early. [ratis]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #1358:
URL: https://github.com/apache/ratis/pull/1358#issuecomment-3948690278
@szetszwo The test `TestRaftAsyncWithNetty.testWithLoadAsync` often times out when running on the server/CI environment. After checking it locally, the only thing I suspect is causing the issue is that this test generates a very large volume of logs. Is it possible to reduce the log output for this specific test (e.g., by adjusting log levels or suppressing certain logs) to help prevent timeouts?
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2417. Fix ChannelMap memory leak in DataStreamManagement when read() fails early. [ratis]
Posted by "slfan1989 (via GitHub)" <gi...@apache.org>.
slfan1989 commented on PR #1358:
URL: https://github.com/apache/ratis/pull/1358#issuecomment-3948699168
> +1 the change looks good.
@szetszwo Thank you very much for taking the time to review the code! This is the last issue I’ve found on the Netty side for now. If I identify any other areas for improvement going forward, I’ll submit additional changes. Next, I’ll focus on the zero-copy aspects of gRPC.
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RATIS-2417. Fix ChannelMap memory leak in DataStreamManagement when read() fails early. [ratis]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #1358:
URL: https://github.com/apache/ratis/pull/1358
--
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: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org