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

    https://github.com/apache/flink/pull/4552#discussion_r162106371
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/PartitionRequestServerHandler.java
 ---
    @@ -82,10 +83,17 @@ protected void channelRead0(ChannelHandlerContext ctx, 
NettyMessage msg) throws
                                LOG.debug("Read channel on {}: {}.", 
ctx.channel().localAddress(), request);
     
                                try {
    -                                   SequenceNumberingViewReader reader = 
new SequenceNumberingViewReader(
    -                                           request.receiverId,
    -                                           request.credit,
    -                                           outboundQueue);
    +                                   NetworkSequenceViewReader reader;
    +                                   if (request.credit > 0) {
    +                                           reader = new 
CreditBasedSequenceNumberingViewReader(
    +                                                   request.receiverId,
    +                                                   request.credit,
    +                                                   outboundQueue);
    +                                   } else {
    +                                           reader = new 
SequenceNumberingViewReader(
    +                                                   request.receiverId,
    +                                                   outboundQueue);
    +                                   }
    --- End diff --
    
    This seems a bit hacky since it does not rely on the configuration 
parameter directly but rather on the affect it has, i.e. that a 
`PartitionRequest` in credit-based flow control always has a non-zero credit 
(due to `NetworkBufferPool#requestMemorySegments()`).
    Relying on the configuration parameter would be nicer, but alternatively, 
you could add a comment clarifying this.


---

Reply via email to