pnowojski commented on a change in pull request #9062: [FLINK-13100][network]
Fix the bug of throwing IOException while FileChannelBoundedData#nextBuffer
URL: https://github.com/apache/flink/pull/9062#discussion_r301975154
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/FileChannelBoundedData.java
##########
@@ -155,6 +159,10 @@ public void close() throws IOException {
@Override
public void recycle(MemorySegment memorySegment) {
buffers.addLast(memorySegment);
+
+ if (!isFinished) {
+ subpartitionView.notifyDataAvailable();
Review comment:
Hmm, something is not right here.
1. there is a cyclic dependency between `FileBufferReader` and
`BoundedBlockingSubpartitionReader`. Here we are calling
`BoundedBlockingSubpartitionReader` which in turns call us back via
`FileBufferReader#nextBuffer`
2. it's strange (when you say it out laud) that we are performing an io read
in `recycle()` call stack
3. it's strange that `notifyDataAvailable` call is doing some io operation
4. it's also a bit strange that `recycle()` call is triggering
`notifyDataAvailable()`, but this maybe unavoidable
5. I think this is showing up in the fact that you have to wrap `IOException`
for 2. and 3. names completely do not suggest what the methods are actually
doing, and it is quite surprising that a light sounding method is doing some
heavy work.
Can not we just notify the view, that we have memory to progress, and we
would make io calls and maybe fill out read ahead buffer during next
`BoundedBlockingSubpartitionReader#getNextBuffer` call? That would fix some of
the problems that I mentioned: it would brake the cycle and there would be no
io operations during recycle/data notification calls.
----------------------------------------------------------------
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
