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

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

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

    https://github.com/apache/flink/pull/5581#discussion_r170921245
  
    --- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/partition/SpillableSubpartitionTest.java
 ---
    @@ -332,56 +290,41 @@ public void 
testConsumeSpillablePartitionSpilledDuringConsume() throws Exception
                // only updated when getting/spilling the buffers but without 
the nextBuffer (kept in memory)
                assertEquals(BUFFER_DATA_SIZE * 3 + 4, 
partition.getTotalNumberOfBytes());
     
    +           // wait for successfully spilling all buffers (before that we 
may not access any spilled buffer and cannot rely on isMoreAvailable!)
                listener.awaitNotifications(2, 30_000);
                assertEquals(2, listener.getNumNotifications());
     
    +           // after consuming and releasing the next buffer, the 
bufferConsumer may be freed,
    +           // depending on the timing of the last write operation
    +           // -> retain once so that we can check below
    +           Buffer buffer = bufferConsumer.build();
    +           buffer.retainBuffer();
    +
                assertFalse(reader.nextBufferIsEvent()); // second buffer 
(retained in SpillableSubpartition#nextBuffer)
    -           read = reader.getNextBuffer();
    -           assertNotNull(read);
    -           assertTrue(read.buffer().isBuffer());
    +           assertNextBuffer(reader, BUFFER_DATA_SIZE, true, 1, true, 
false);
                assertEquals(BUFFER_DATA_SIZE * 4 + 4, 
partition.getTotalNumberOfBytes()); // finally integrates the nextBuffer 
statistics
                assertEquals(1, partition.getBuffersInBacklog());
    -           assertEquals(partition.getBuffersInBacklog(), 
read.buffersInBacklog());
    -           read.buffer().recycleBuffer();
    -           // now the bufferConsumer may be freed, depending on the timing 
of the write operation
    -           // -> let's do this check at the end of the test (to save some 
time)
    -           assertTrue(read.nextBufferIsEvent());
    +
    +           bufferConsumer.close(); // recycle the retained buffer from 
above (should be the last reference!)
     
                assertTrue(reader.nextBufferIsEvent()); // the event (spilled)
    -           read = reader.getNextBuffer();
    -           assertNotNull(read);
    -           assertFalse(read.buffer().isBuffer());
    +           assertNextEvent(reader, BUFFER_DATA_SIZE, null, true, 1, false, 
true);
    --- End diff --
    
    ditto


> SpilledSubpartitionView wrongly relys on the backlog for determining whether 
> more data is available
> ---------------------------------------------------------------------------------------------------
>
>                 Key: FLINK-8755
>                 URL: https://issues.apache.org/jira/browse/FLINK-8755
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Network
>            Reporter: Nico Kruber
>            Assignee: Nico Kruber
>            Priority: Blocker
>             Fix For: 1.5.0
>
>
> {code}
> public BufferAndBacklog getNextBuffer() throws IOException, 
> InterruptedException {
> //...
>         int newBacklog = parent.decreaseBuffersInBacklog(current);
>         return new BufferAndBacklog(current, newBacklog > 0, newBacklog, 
> nextBufferIsEvent);
> {code}
> relies on the backlog to signal further data availability. However, if there 
> are only events left in the buffer queue, their buffers are not included in 
> the backlog counting and therefore, {{isMoreAvailable}} will be wrongly 
> {{false}} here.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to