zhijiangW commented on issue #8557: [FLINK-12544][network] Fix the deadlock while releasing memory triggered by multiple threads in SpillableSubpartition URL: https://github.com/apache/flink/pull/8557#issuecomment-497020214 Thanks for the reviews and good suggestions @pnowojski ! Actually I also had the same performance concern as you when refactor the `LocalBufferPool`. The final decision is based on below considerations: - It is buffer-level overhead which might be ignored. - The second `synchronized` only happens during backpressure as you mentioned, so it seems no matter in this case. - If we adjust the `SpillableSubpartition`, it seems more safe because it is rarely used and only for batch mode. But the internal logics in `SpillableSubpartition` seem a bit messy, and we should be very careful in order not to bring other issues. In contrast, the logic in `LocalBufferPool` seems more clearly to go. And it is very obvious the owner release should not be inside `synchronized`. In addition, I think the process of `owner.get().releaseMemory(1)` could be improved a bit. The `releaseMemory` could return a int value for indicating how many buffers are released. If the return value is more than 0, we could try get available buffer directly after that, no need to enter `wait(2000)` for blocking mode or return null directly. But it is a different topic not related to the deadlock issue. And in the latest master code, the `releaseMemory` is invalid because it would always return 0 for the new `BoundedBlockingSubpartition`. So I think it does not worth efforts for improving it. If you still want to try another way of adjusting the `SpillableSubpartition`, I could further think and have a try.
---------------------------------------------------------------- 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
