[GitHub] [flink] AHeise commented on a change in pull request #13709: [FLINK-19401][checkpointing] Download checkpoints only if needed

[GitBox <gi...@apache.org>]

AHeise commented on a change in pull request #13709:
URL: https://github.com/apache/flink/pull/13709#discussion_r509211683



##########
File path: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/ZooKeeperCompletedCheckpointStore.java
##########
@@ -143,6 +145,10 @@ public void recover() throws Exception {
 		int numberOfInitialCheckpoints = initialCheckpoints.size();
 
 		LOG.info("Found {} checkpoints in ZooKeeper.", numberOfInitialCheckpoints);
+		if (haveAllDownloaded(initialCheckpoints)) {

Review comment:
       What happens if all but one checkpoint have been downloaded? Would it download all of them or just the missing one?
   If the former, I'd modify this method to return all missing checkpoints instead.

##########
File path: flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/ZooKeeperCompletedCheckpointStoreTest.java
##########
@@ -58,6 +66,54 @@ public void testPathConversion() {
 		assertEquals(checkpointId, ZooKeeperCompletedCheckpointStore.pathToCheckpointId(path));
 	}
 
+	private static class TestException extends RuntimeException{

Review comment:
       Could we reuse `ExpectedTestException` from `runtime` `testutils` package?




----------------------------------------------------------------
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:
users@infra.apache.org