gaoyunhaii commented on pull request #16800:
URL: https://github.com/apache/flink/pull/16800#issuecomment-898438853
Very thanks Piotr @pnowojski for updating the PR! The new method looks good
to me. The only concern is that there might be repeat decline some times
logically, but in realistic the `StreamTask` only declines the checkpoints if
`isRunning = false`, which should not happen due to the same reason as the
above comment, and even if there are repeat decline there should be no
problems.
And since now the change is limited to Task, perhaps we could add some UT in
`TaskTest` to check the decline is indeed called in the three places? Might be
something like:
```java
public void testDeclineCheckpointIfTaskIsNotRunning() throws Exception {
TestCheckpointResponder testCheckpointResponder = new
TestCheckpointResponder();
final Task task =
createTaskBuilder().setCheckpointResponder(testCheckpointResponder).build();
task.triggerCheckpointBarrier(
1,
1,
CheckpointOptions.alignedNoTimeout(
CheckpointType.CHECKPOINT,
CheckpointStorageLocationReference.getDefault()));
assertEquals(1, testCheckpointResponder.getDeclineReports().size());
assertEquals(
CheckpointFailureReason.CHECKPOINT_DECLINED_TASK_NOT_READY,
testCheckpointResponder
.getDeclineReports()
.get(0)
.getCause()
.getCheckpointFailureReason());
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
