Github user zhijiangW commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4485#discussion_r139083473
  
    --- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/buffer/NetworkBufferPoolTest.java
 ---
    @@ -172,44 +178,117 @@ public void testDestroyAll() {
                }
        }
     
    +   /**
    +    * Tests {@link NetworkBufferPool#requestMemorySegments(int)} with the 
{@link NetworkBufferPool}
    +    * currently containing the number of required free segments.
    +    */
        @Test
    -   public void testRequestAndRecycleMemorySegments() throws Exception {
    +   public void testRequestMemorySegmentsLessThanTotalBuffers() throws 
Exception {
                final int numBuffers = 10;
     
                NetworkBufferPool globalPool = new 
NetworkBufferPool(numBuffers, 128, MemoryType.HEAP);
     
    -           List<MemorySegment> segments = null;
    -           // request buffers from global pool with illegal argument
    +           List<MemorySegment> memorySegments = Collections.emptyList();
                try {
    -                   segments = globalPool.requestMemorySegments(0);
    -                   fail("Should throw an IllegalArgumentException");
    -           } catch (IllegalArgumentException e) {
    -                   assertNull(segments);
    +                   memorySegments = 
globalPool.requestMemorySegments(numBuffers / 2);
    +
    +                   assertEquals(memorySegments.size(), numBuffers / 2);
    +           } finally {
    +                   globalPool.recycleMemorySegments(memorySegments);
                        
assertEquals(globalPool.getNumberOfAvailableMemorySegments(), numBuffers);
                }
    +   }
     
    -           // common case to request buffers less than the total capacity 
of global pool
    -           final int numRequiredBuffers = 8;
    -           segments = globalPool.requestMemorySegments(numRequiredBuffers);
    -
    -           assertNotNull(segments);
    -           assertEquals(segments.size(), numRequiredBuffers);
    -
    -           // recycle all the requested buffers to global pool
    -           globalPool.recycleMemorySegments(segments);
    +   /**
    +    * Tests {@link NetworkBufferPool#requestMemorySegments(int)} with the 
number of required
    +    * buffers exceeding the capacity of {@link NetworkBufferPool}.
    +    */
    +   @Test
    +   public void testRequestMemorySegmentsMoreThanTotalBuffers() throws 
Exception {
    +           final int numBuffers = 10;
     
    -           assertEquals(globalPool.getNumberOfAvailableMemorySegments(), 
numBuffers);
    +           NetworkBufferPool globalPool = new 
NetworkBufferPool(numBuffers, 128, MemoryType.HEAP);
     
    -           // uncommon case to request buffers exceeding the total 
capacity of global pool
    +           List<MemorySegment> memorySegments = Collections.emptyList();
                try {
    -                   segments = null;
    -                   segments = globalPool.requestMemorySegments(11);
    +                   memorySegments = 
globalPool.requestMemorySegments(numBuffers + 1);
                        fail("Should throw an IOException");
                } catch (IOException e) {
    -                   assertNull(segments);
    -                   // recycle all the requested buffers to global pool 
after exception
    +                   assertEquals(memorySegments.size(), 0);
                        
assertEquals(globalPool.getNumberOfAvailableMemorySegments(), numBuffers);
                }
    +   }
     
    +   /**
    +    * Tests {@link NetworkBufferPool#requestMemorySegments(int)} with the 
invalid argument to
    +    * cause exception.
    +    */
    +   @Test
    +   public void testRequestMemorySegmentsWithInvalidArgument() throws 
Exception {
    +           final int numBuffers = 10;
    +
    +           NetworkBufferPool globalPool = new 
NetworkBufferPool(numBuffers, 128, MemoryType.HEAP);
    +
    +           List<MemorySegment> memorySegments = Collections.emptyList();
    +           try {
    +                   // the number of requested buffers should be larger 
than zero
    +                   memorySegments = globalPool.requestMemorySegments(0);
    +                   fail("Should throw an IllegalArgumentException");
    +           } catch (IllegalArgumentException e) {
    +                   assertEquals(memorySegments.size(), 0);
    +                   
assertEquals(globalPool.getNumberOfAvailableMemorySegments(), numBuffers);
    +           }
    +   }
    +
    +   /**
    +    * Tests {@link NetworkBufferPool#requestMemorySegments(int)} with the 
{@link NetworkBufferPool}
    +    * currently not containing the number of required free segments 
(currently occupied by a buffer pool).
    +    */
    +   @Test
    +   public void testRequestMemorySegmentsWithBuffersTaken() throws 
IOException, InterruptedException {
    +           final int numBuffers = 10;
    +
    +           NetworkBufferPool networkBufferPool = new 
NetworkBufferPool(numBuffers, 128, MemoryType.HEAP);
    +
    +           final List<Buffer> buffers = new ArrayList<>(numBuffers);
    +           List<MemorySegment> memorySegments = Collections.emptyList();
    +           Thread bufferRecycler = null;
    +           BufferPool lbp1 = null;
    +           try {
    +                   lbp1 = networkBufferPool.createBufferPool(numBuffers / 
2, numBuffers);
    +
    +                   // take all buffers (more than the minimum required)
    +                   for (int i = 0; i < numBuffers; ++i) {
    +                           Buffer buffer = lbp1.requestBuffer();
    +                           buffers.add(buffer);
    +                           assertNotNull(buffer);
    +                   }
    +
    +                   // if requestMemorySegments() blocks, this will make 
sure that enough buffers are freed
    +                   // eventually for it to continue
    +                   bufferRecycler = new Thread(() -> {
    +                           try {
    +                                   Thread.sleep(10000);
    --- End diff --
    
    agree


---

Reply via email to