dawidwys commented on pull request #14820:
URL: https://github.com/apache/flink/pull/14820#issuecomment-854630242


   Honestly, I am not convinced about the changes. I don't like exposing the 
`CheckpointBarrierHandler`. Moreover the `triggerCheckpoint` method does not 
fit well into the class imo.
   
   You already have two separate methods in the `StreamTask` for triggering a 
checkpoint either from RPC or on a barrier. I don't necessarily understand why 
does it have to do so many rounds through the 
`StreamTask#triggerCheckpointAsync` -> 
`CheckpointBarrierHandler#triggerCheckpoint` -> 
`StreamTask#triggerCheckpointOnBarrier` -> `StreamTask#performCheckpoint`. 
Can't you just remove the two middle man? I think a check if all inputs are 
finished in the `StreamTask#triggerCheckpointAsync` of non-source tasks should 
be enough.


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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to