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

    https://github.com/apache/flink/pull/5802#discussion_r179453256
  
    --- Diff: 
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterTest.java
 ---
    @@ -309,8 +309,6 @@ public void testBroadcastEventBufferReferenceCounting() 
throws Exception {
                for (int i = 0; i < queues.length; i++) {
                        assertTrue(parseBuffer(queues[i].remove(), 
i).isEvent());
                }
    -
    -           assertTrue(bufferConsumer.isRecycled());
        }
    --- End diff --
    
    yes, this should not be dropped since this line is the actual test - how 
about this instead:
    ```
                // Verify added to all queues
                assertEquals(1, queues[0].size());
                assertEquals(1, queues[1].size());
                
                // get references to buffer consumers (copies from the original 
event buffer consumer)
                BufferConsumer bufferConsumer1 = queues[0].getFirst();
                BufferConsumer bufferConsumer2 = queues[1].getFirst();
    
                // process all collected events (recycles the buffer)
                assertTrue(parseBuffer(queues[0].remove(), 0).isEvent());
                assertTrue(parseBuffer(queues[1].remove(), 1).isEvent());
    
                assertTrue(bufferConsumer1.isRecycled());
                assertTrue(bufferConsumer2.isRecycled());
    ```


---

Reply via email to