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

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

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

    https://github.com/apache/flink/pull/4533#discussion_r146178609
  
    --- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/CreditBasedClientHandler.java
 ---
    @@ -269,4 +315,49 @@ private void decodeBufferOrEvent(RemoteInputChannel 
inputChannel, NettyMessage.B
                        bufferOrEvent.releaseBuffer();
                }
        }
    +
    +   private void writeAndFlushNextMessageIfPossible(Channel channel) {
    +           if (channelError.get() != null || !channel.isWritable()) {
    +                   return;
    +           }
    +
    +           while (true) {
    +                   RemoteInputChannel inputChannel = 
inputChannelsWithCredit.poll();
    +
    +                   // The input channel may be null because of the write 
callbacks that are executed
    +                   // after each write, and it is also no need to notify 
credit for released channel.
    +                   if (inputChannel == null || inputChannel.isReleased()) {
    +                           return;
    +                   }
    +
    +                   AddCredit msg = new AddCredit(
    +                           inputChannel.getPartitionId(),
    +                           inputChannel.getAndResetCredit(),
    +                           inputChannel.getInputChannelId());
    +
    +                   // Write and flush and wait until this is done before
    +                   // trying to continue with the next input channel.
    +                   channel.writeAndFlush(msg).addListener(writeListener);
    +
    +                   return;
    --- End diff --
    
    Firsts of all sorry I made a mistake in my previous comment: 
    > However because `channel.isWritable()` returned true
    
    was supposed to be:
    > However because `channel.isWritable()` returned false
    
    > Second, once writeAndFlushNextMessageIfPossible is called, it will loop 
all the InputChannel s in the queue until the channel becomes un-writable. Then 
the left InputChannel s in the queue will be chosen after the channel becomes 
writable again.
    
    Can you point out what code will loop through those `InputChannel`s? Can 
you please take a careful look at the `writeAndFlushNextMessageIfPossible`, 
especially both exit points from the `while (true)` loop? This code completely 
doesn't make sense to me.
    
    ```
    while (true) { // <<========= ?
        RemoteInputChannel inputChannel = inputChannelsWithCredit.poll();
    
        // The input channel may be null because of the write callbacks that 
are executed
        // after each write, and it is also no need to notify credit for 
released channel.
        if (inputChannel == null || inputChannel.isReleased()) {
                return;
        }
        (...)
    
        return; // <<========= ?
    }
    ```


> Implement Netty receiver outgoing pipeline for credit-based
> -----------------------------------------------------------
>
>                 Key: FLINK-7416
>                 URL: https://issues.apache.org/jira/browse/FLINK-7416
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Network
>            Reporter: zhijiang
>            Assignee: zhijiang
>             Fix For: 1.4.0
>
>
> This is a part of work for credit-based network flow control.
> The related works are :
> *  We define a new message called {{AddCredit}} to notify the incremental 
> credit during data shuffle. 
> * Whenever an {{InputChannel}}’s unannounced credit goes up from zero, the 
> channel is enqueued in the pipeline.
> * Whenever the channel becomes writable, it takes the next {{InputChannel}} 
> and sends its unannounced credit. The credit is reset to zero after each sent.
> * That way, messages are sent as often as the network has capacity and 
> contain as much credit as available for the channel at that point in time. 
> Otherwise, it would only add latency to the announcements and not increase 
> throughput.



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

Reply via email to