pnowojski commented on a change in pull request #12261:
URL: https://github.com/apache/flink/pull/12261#discussion_r428179383
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/RemoteInputChannel.java
##########
@@ -181,6 +181,14 @@ void retriggerSubpartitionRequest(int subpartitionIndex)
throws IOException {
moreAvailable = !receivedBuffers.isEmpty();
}
+ if (next == null) {
Review comment:
What do you mean @zhijiangW ? At first glance I would agree with
@Jiayi-Liao, that it shouldn't happen after your fix in this commit in this
class below.
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/consumer/RemoteInputChannelTest.java
##########
@@ -1010,6 +1011,56 @@ public void testConcurrentRecycleAndRelease2() throws
Exception {
}
}
+ @Test
+ public void testConcurrentGetNextBufferAndRelease() throws Exception {
Review comment:
I think we could have tested this bug without concurrency/multi
threading by just calling `releaseAllResources` before `getNextBuffer`. Test
would be much easier to understand and debug and this would be worth a bit
worse testing coverage - especially that we still have ITCases, and in the long
run we are planning/hoping to make releasing resources go through mailbox.
----------------------------------------------------------------
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]
