pnowojski commented on a change in pull request #12120:
URL: https://github.com/apache/flink/pull/12120#discussion_r426411404
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/disk/FileBasedBufferIterator.java
##########
@@ -85,6 +90,6 @@ private int read(byte[] buffer) {
@Override
public void close() throws Exception {
- closeAll(stream, file::release);
+ closeAll(stream, file::release, () ->
buffersToClose.forEach(Buffer::recycleBuffer));
Review comment:
> The client doesn't know whether the iterator is lazy or not. So if
failure happens before iteration, it would have to read the whole file just to
recycle the buffers.
Why? As I wrote before, the buffers that were not yet returned, should be
recycled by it's current owner - in that case `CloseableIterator`. If the
buffers were even not yet created, because it's a lazy iterator, even better -
nothing to do in the `close()`.
> In the beginnning of FileBasedBufferIterator.next().
If buffers were returned from the iterator, iterator is no long it's owner
and they shouldn't be recycled by the iterator.
----------------------------------------------------------------
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]
