akalash commented on a change in pull request #18865:
URL: https://github.com/apache/flink/pull/18865#discussion_r813088119
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/buffer/NetworkBufferPoolTest.java
##########
@@ -637,13 +672,17 @@ public void go() throws Exception {
numberOfSegmentsToRequest));
}
};
- asyncRequest.start();
// recycle 5 segments
CompletableFuture<?> availableFuture =
globalPool.getAvailableFuture();
globalPool.recycleUnpooledMemorySegments(segments1);
assertTrue(availableFuture.isDone());
+ // Starting to requesting next segments only after previous
segments were release
+ // otherwise the race condition between requesting new segments
and releasing old ones
+ // are possible:
+ asyncRequest.start();
Review comment:
Actually, right now I see that this test was incorrect. This test
supposed that `requestUnpooledMemorySegments` would be blocked until enough
buffers will be available but it is just not true. I don't know it was the
initial mistake in the test or the old behavior was different. Anyway I will
remove this extra thread that doesn't make sense anymore.
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/buffer/NetworkBufferPoolTest.java
##########
@@ -292,6 +284,52 @@ public void
testRequestMemorySegmentsMoreThanTotalBuffers() {
}
}
+ /**
+ * Tests {@link NetworkBufferPool#requestUnpooledMemorySegments(int)} with
the total number of
+ * allocated buffers for several requests exceeding the capacity of {@link
NetworkBufferPool}.
+ */
+ @Test
+ public void testInsufficientNumberOfBuffers() throws Exception {
+ final int numberOfSegmentsToRequest = 5;
+
+ final NetworkBufferPool globalPool = new
NetworkBufferPool(numberOfSegmentsToRequest, 128);
+
+ try {
+ // the global pool should be in available state initially
+ assertTrue(globalPool.getAvailableFuture().isDone());
+
+ // request 5 segments
+ List<MemorySegment> segments1 =
+
globalPool.requestUnpooledMemorySegments(numberOfSegmentsToRequest);
+ assertFalse(globalPool.getAvailableFuture().isDone());
+ assertEquals(numberOfSegmentsToRequest, segments1.size());
+
+ try {
+ // request another 5 segments
+
globalPool.requestUnpooledMemorySegments(numberOfSegmentsToRequest);
Review comment:
Yes, perhaps using the one instead of five is good idea
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
