1996fanrui commented on code in PR #24246: URL: https://github.com/apache/flink/pull/24246#discussion_r1475645896
########## flink-runtime/src/test/java/org/apache/flink/runtime/testutils/CommonTestUtils.java: ########## @@ -367,9 +367,11 @@ public static void waitForCheckpoint(JobID jobID, MiniCluster miniCluster, int n }); } - /** Wait for on more completed checkpoint. */ - public static void waitForOneMoreCheckpoint(JobID jobID, MiniCluster miniCluster) - throws Exception { + /** + * Wait for a new completed checkpoint. Note: we wait for 2 or more checkpoint to ensure the + * latest checkpoint start after waitForTwoOrMoreCheckpoint is called. + */ + public static void waitForNewCheckpoint(JobID jobID, MiniCluster miniCluster) throws Exception { Review Comment: > That way, we can pass in 2 in the test implementation analogously to waitForCheckpoint. I also feel like we can remove some redundant code within the two methods. 🤔 IIUC, the semantic between `waitForCheckpoint` and `waitForOneMoreCheckpoint` are different. (`waitForOneMoreCheckpoint` is renamed to `waitForNewCheckpoint` in this PR.) - `waitForCheckpoint` check the total count of all completed checkpoints. - `waitForOneMoreCheckpoint` check the whether the new checkpoint is completed after it's called. - For example, the job has 10 completed checkpoint before it's called. - `waitForOneMoreCheckpoint` will wait for checkpoint-11 is completed. BTW, I have refactored the `waitForNewCheckpoint`. I check the checkpoint trigger time instead of checkpointCount. I think checking trigger time is clearer than checkpointCount >= 2. Other developers might don't know why check 2 checkpoint here, and `checkpointCount >= 2` doesn't work when enabling the concurrent checkpoint. WDYT? -- 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