[GitHub] flink pull request #4581: [FLINK-7499][io] also let AsynchronousBufferFileWr...

[pnowojski <gi...@git.apache.org>]

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

    https://github.com/apache/flink/pull/4581#discussion_r148532507
  
    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/SpillableSubpartitionTest.java ---
    @@ -192,21 +198,39 @@ public void testConsumeSpilledPartition() throws Exception {
     
     		Buffer read = reader.getNextBuffer();
     		assertNotNull(read);
    +		assertNotSame(buffer, read);
    +		assertFalse(read.isRecycled());
     		read.recycle();
    +		assertTrue(read.isRecycled());
     
     		read = reader.getNextBuffer();
     		assertNotNull(read);
    +		assertNotSame(buffer, read);
    +		assertFalse(read.isRecycled());
     		read.recycle();
    +		assertTrue(read.isRecycled());
     
     		read = reader.getNextBuffer();
     		assertNotNull(read);
    +		assertNotSame(buffer, read);
    +		assertFalse(read.isRecycled());
     		read.recycle();
    +		assertTrue(read.isRecycled());
     
     		// End of partition
     		read = reader.getNextBuffer();
     		assertNotNull(read);
     		assertEquals(EndOfPartitionEvent.class, EventSerializer.fromBuffer(read, ClassLoader.getSystemClassLoader()).getClass());
    +		assertFalse(read.isRecycled());
     		read.recycle();
    +		assertTrue(read.isRecycled());
    +
    +		// finally check that the buffer has been freed after a successful (or failed) write
    +		final long deadline = System.currentTimeMillis() + 30_000L; // 30 secs
    +		while (!buffer.isRecycled() && System.currentTimeMillis() < deadline) {
    +			Thread.sleep(1);
    +		}
    +		assertTrue(buffer.isRecycled());
    --- End diff --
    
    Where is the last place in this test were you can add `assertFalse(buffer.isRecycled())`? I think placing it somewhere would help to understand what's going on and would be nice sanity check.
    
    ~~Isn't the `buffer` recycled in `partition.releaseMemory()` call far above?~~ yes it is ;)


---