zhijiangW commented on a change in pull request #12261:
URL: https://github.com/apache/flink/pull/12261#discussion_r429018568
##########
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 thought of some other considerations for this issue to share.
In the ITCase, even though we can reproduce some potential concurrent bugs,
it is hard to debug and find the root cause, because it is involved in all the
components. I really have such feeling when debugging the
`UnalignedCheckpointITCase` these days.
Reversely, unit test only works on two concurrent methods directly, so it is
easy to find the bugs by limiting the scopes/components. We already had 6 unit
tests written by concurrent way in `RemoteInputChannelTest` before, to
guarantee the stability among different concurrent methods executed by task
thread, netty thread, canceler thread separately. If replaced by ITCase, we
need to debug among all these methods to find the potential root cause.
In general, it is better for unit tests only focus on one component or less,
otherwise we should rely on ITCase. In this case, we only limit the scope
inside `RemoteInputChannel` component, so it also makes sense from this aspect.
Anyway besides the pros I mentioned above for unit tests, I also agree that
the cons you concerned, merely the pros are a bit more than cons on my side. :)
----------------------------------------------------------------
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:
us...@infra.apache.org
