HeartSaVioR commented on a change in pull request #27064:
[SPARK-30246]OneForOneStreamManager might leak memory in connectionTerminated
URL: https://github.com/apache/spark/pull/27064#discussion_r362651186
##########
File path:
common/network-common/src/main/java/org/apache/spark/network/server/OneForOneStreamManager.java
##########
@@ -117,19 +118,26 @@ public static String genStreamChunkId(long streamId, int
chunkId) {
@Override
public void connectionTerminated(Channel channel) {
+ LinkedList<StreamState> removedStates = new LinkedList<>();
// Close all streams which have been associated with the channel.
for (Map.Entry<Long, StreamState> entry: streams.entrySet()) {
StreamState state = entry.getValue();
if (state.associatedChannel == channel) {
- streams.remove(entry.getKey());
+ removedStates.add(streams.remove(entry.getKey()));
+ }
+ }
- // Release all remaining buffers.
+ for (StreamState state: removedStates) {
+ // Release all remaining buffers.
+ try {
while (state.buffers.hasNext()) {
ManagedBuffer buffer = state.buffers.next();
Review comment:
So I think there're two options here with achieving goal;
1. store any exception between exceptions (as we'll try to release from all
the streams) and rethrow, then channelInactive will log it. other exceptions
should be logged as well but maybe at here (or a new exception containing all
exceptions and throw it instead).
2. catch them, and log and swallow at here.
Technically the change from logging side would be logger name which may not
a big deal, but it's also valid concern if we think channelInactive is the one
to decide how to handle the exception.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
