You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2021/10/14 07:19:10 UTC

[GitHub] [flink] pnowojski commented on a change in pull request #17471: [FLINK-24465] Fix disabling buffer timeout for pipelined exchanges

pnowojski commented on a change in pull request #17471:
URL: https://github.com/apache/flink/pull/17471#discussion_r728699504



##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGenerator.java
##########
@@ -105,9 +105,7 @@
 
     private static final Logger LOG = LoggerFactory.getLogger(StreamingJobGraphGenerator.class);
 
-    private static final long DEFAULT_NETWORK_BUFFER_TIMEOUT = 100L;
-
-    public static final long UNDEFINED_NETWORK_BUFFER_TIMEOUT = -1L;
+    public static final long DISABLED_NETWORK_BUFFER_TIMEOUT = -1L;

Review comment:
       Since you are already touching/modifing this, I would define this constant in `RecordWriter` and use it there as well.

##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/environment/StreamExecutionEnvironment.java
##########
@@ -176,7 +176,7 @@
 

Review comment:
       Can you copy paste PR description to the commit message?
   ```
   The goal is to fix the possibility to disable buffer timeout for pipelined exchanges. It has been broken in #13209, which was supposed to add a check that buffer timeout is properly configured if blocking exchanges are present in the graph.
   
   Brief change log
   treat -1 again as a valid configuration instead of UNDEFINED
   it disables the automatic disabling of buffer timeout for blocking exchanges
   ```

##########
File path: flink-streaming-java/src/main/java/org/apache/flink/streaming/api/graph/StreamingJobGraphGenerator.java
##########
@@ -105,9 +105,7 @@
 
     private static final Logger LOG = LoggerFactory.getLogger(StreamingJobGraphGenerator.class);
 
-    private static final long DEFAULT_NETWORK_BUFFER_TIMEOUT = 100L;
-
-    public static final long UNDEFINED_NETWORK_BUFFER_TIMEOUT = -1L;
+    public static final long DISABLED_NETWORK_BUFFER_TIMEOUT = -1L;

Review comment:
       Since you are already touching/modifing this, I would define this constant in `RecordWriter` and use it there as well?




-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

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