[GitHub] [flink] curcur edited a comment on pull request #12353: [FLINK-17322][network] Fixes BroadcastRecordWriter overwriting memory segments on first finished BufferConsumer.

[GitBox <gi...@apache.org>]

curcur edited a comment on pull request #12353:
URL: https://github.com/apache/flink/pull/12353#issuecomment-635161998


   Hey @AHeise , I've spent some time walking through the code. I think the bug is clear that the consumer's  reference counter of `bufferBuilder` for `randomEmit` is not counted correctly. But I was wondering whether this fix work as expected in the case if the `record` to `randomEmit` is more than one buffer can hold.
   
   In `BroadcastRandomWriter#randomEmit`, random triggered data is emitted in the line of `emit(record, targetChannelIndex);`
   
   If you trace the code down, eventually data is serialized through `RecordWriter#copyFromSerializerToTargetChannel`
   
   and there, `requestNewBufferBuilder` is requested multiple times if the data to serialize is more than one buffer can hold.
   
   And as you can see, all the references to the remaining bufferBuilders are lost. Since the `addConsumer` in `randomEmit` is done after `emit(record, targetChannelIndex)`, it looks like so from the code perspective.
   
   But I am not quite familiar with this part of code, not sure whether this will cause real problems. 
   
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org