[ 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)