akalash commented on a change in pull request #16637:
URL: https://github.com/apache/flink/pull/16637#discussion_r685828704



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointFailureManager.java
##########
@@ -136,6 +135,8 @@ public void checkFailureCounter(CheckpointException 
exception, long checkpointId
                 // ignore
                 break;
 
+            case EXCEPTION:

Review comment:
       @AHeise In this PR new `CheckpointFailureReason` is 
introduced(`IO_EXCEPTION`). Now we want to replace the existing `EXCEPTION` 
with `IO_EXCEPTION`(because according to the code it is exactly IO_EXCEPTION). 
Also, `IO_EXCEPTION` would not be ignored by `FailureManager` as it was with 
`EXCEPTION`. Do you have any objections to this?
   
   @zlzhang0122 If nobody will mind, I suggest getting rid of the EXCEPTION and 
replace it with IO_EXCEPTION. But it is better to do so in two separated 
commits(under one ticket). As a result of this ticket, you will have two 
commits - one of them adding the IO_EXCEPTION but don't touch the EXCEPTION and 
the second one just replace EXCEPTION with IO_EXCEPTION.




-- 
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-unsubscr...@flink.apache.org

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


Reply via email to