flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From NicoK <...@git.apache.org>
Subject [GitHub] flink pull request #4533: [FLINK-7416][network] Implement Netty receiver out...
Date Tue, 28 Nov 2017 11:24:05 GMT
Github user NicoK commented on a diff in the pull request:

    https://github.com/apache/flink/pull/4533#discussion_r153446896
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/CreditBasedClientHandler.java
---
    @@ -102,13 +102,21 @@ void cancelRequestFor(InputChannelID inputChannelId) {
     		}
     	}
     
    +	/**
    +	 * The credit is announced based on sender's backlog from buffer response, so the channel
is
    +	 * already active then. If the channel is closed because of error, we can skip this
announcement.
    +	 *
    +	 * @param inputChannel The input channel with unannounced credits.
    +	 */
     	void notifyCreditAvailable(final RemoteInputChannel inputChannel) {
    -		ctx.executor().execute(new Runnable() {
    -			@Override
    -			public void run() {
    -				ctx.pipeline().fireUserEventTriggered(inputChannel);
    -			}
    -		});
    +		if (ctx != null) {
    --- End diff --
    
    I actually preferred the previous implementation because it would throw in case of invalid
use, i.e. if the channel had not been active before. Or is there a use case for `ctx == null`
that is valid and where we really should skip credit notification? But recall since we are
only ever calling this when the credit moves from 0 to 1 and therefore, if we ever skip one
notification, we will not be notified again!


---

Mime
View raw message