zhijiangW commented on a change in pull request #7549: [FLINK-11403][network]
Remove ResultPartitionConsumableNotifier from ResultPartition
URL: https://github.com/apache/flink/pull/7549#discussion_r267177872
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/ResultPartitionTest.java
##########
@@ -148,15 +148,17 @@ public void testAddOnReleasedBlockingPartition() throws
Exception {
/**
* Tests {@link ResultPartition#addBufferConsumer} on a partition which
has already been released.
*
- * @param pipelined the result partition type to set up
+ * @param partitionType the result partition type to set up
*/
- protected void testAddOnReleasedPartition(final ResultPartitionType
pipelined)
+ protected void testAddOnReleasedPartition(final ResultPartitionType
partitionType)
throws Exception {
BufferConsumer bufferConsumer =
createFilledBufferConsumer(BufferBuilderTestUtils.BUFFER_SIZE);
ResultPartitionConsumableNotifier notifier =
mock(ResultPartitionConsumableNotifier.class);
try {
- ResultPartition partition = createPartition(notifier,
pipelined, true);
- partition.release();
+ NetworkResultPartition networkPartition =
createNetworkResultPartition(partitionType);
+ ResultPartition partition = new ResultPartition(
Review comment:
I think we might still need the `ResultPartition`.
This test wants to verify the notification logic after releasing partition.
The `partition.addBufferConsumer` would return false after released, then it
would never trigger notification which is verified in finally path. And the
notification logic is removed from `networkPartition`, so it can not be
verified directly.
----------------------------------------------------------------
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
With regards,
Apache Git Services
