pnowojski commented on a change in pull request #6809: [FLINK-10491][network]
Pass BufferPoolOwner in the constructor of LocalBufferPool
URL: https://github.com/apache/flink/pull/6809#discussion_r226288887
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/ResultPartitionTest.java
##########
@@ -205,6 +216,66 @@ protected void testAddOnPartition(final
ResultPartitionType pipelined)
}
}
+ @Test
+ public void testReleaseMemoryOnBlockingPartition() throws Exception {
+ testReleaseMemory(ResultPartitionType.BLOCKING);
+ }
+
+ @Test
+ public void testReleaseMemoryOnPipelinedPartition() throws Exception {
+ testReleaseMemory(ResultPartitionType.PIPELINED);
+ }
+
+ /**
+ * Tests {@link ResultPartition#releaseMemory(int)} on a working
partition.
+ *
+ * @param resultPartitionType the result partition type to set up
+ */
+ private void testReleaseMemory(final ResultPartitionType
resultPartitionType) throws Exception {
+ final int numBuffers = 10;
+ final NetworkEnvironment network = new NetworkEnvironment(
Review comment:
Could you deduplicate this constructor with other use cases? It is used in
tests in 5 other places, 2 of them are almost exactly the same as this one and
three others further duplicate this:
```
new LocalConnectionManager(),
new ResultPartitionManager(),
new TaskEventDispatcher(),
new KvStateRegistry(),
null,
null,
IOManager.IOMode.SYNC,
```
block.
Please either provide some more constructors to deduplicate this or provide
a `NetworkEnvironmentBuilder` (or `public static class Builder` nested in
`NetworkEnvironment`) that would handle those default values. I would
personally prefer the builder, since it's better for the future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
