[ 
https://issues.apache.org/jira/browse/FLINK-7499?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16264394#comment-16264394
 ] 

ASF GitHub Bot commented on FLINK-7499:
---------------------------------------

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

    https://github.com/apache/flink/pull/4581#discussion_r152813605
  
    --- 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 --
    
    No, it's not recycled in `partition.releaseMemory()` directly (or at least 
should not! - which is fixed now). The buffer will be recycled by the 
asynchronous writer thread once its content has been written to disk, i.e. 
after the `partition.releaseMemory()` call - I included such a check right 
before that call but actually, this is of limited use.


> double buffer release in SpillableSubpartitionView
> --------------------------------------------------
>
>                 Key: FLINK-7499
>                 URL: https://issues.apache.org/jira/browse/FLINK-7499
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Network
>    Affects Versions: 1.2.0, 1.1.4, 1.3.0, 1.1.5, 1.2.1, 1.2.2, 1.3.1, 1.4.0, 
> 1.3.2, 1.3.3
>            Reporter: Nico Kruber
>            Assignee: Nico Kruber
>            Priority: Blocker
>
> {{SpillableSubpartitionView#releaseMemory()}} recycles its buffers twice: 
> once asynchronously after the write operation and once in 
> {{SpillableSubpartitionView#releaseMemory()}} after adding the write 
> operation to the queue.
> 1) if {{SpillableSubpartitionView#releaseMemory()}} hits first and the buffer 
> is recycled, the memory region may already be reused despite the pending write
> 2) If, for some reason (probably only in tests like 
> {{SpillableSubpartitionTest#testConsumeSpillablePartitionSpilledDuringConsume()}}?),
>  the buffer is retained and to be used in parallel somewhere else it may also 
> not be available anymore or contain corrupt data.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to