Github user StefanRRichter commented on a diff in the pull request:
https://github.com/apache/flink/pull/4192#discussion_r126725439
--- Diff:
flink-runtime/src/test/java/org/apache/flink/runtime/checkpoint/ZooKeeperCompletedCheckpointStoreTest.java
---
@@ -160,9 +162,12 @@ public Void answer(InvocationOnMock invocation) throws
Throwable {
stateStorage,
Executors.directExecutor());
- SharedStateRegistry sharedStateRegistry = new
SharedStateRegistry();
+ SharedStateRegistry sharedStateRegistry = spy(new
SharedStateRegistry());
zooKeeperCompletedCheckpointStore.recover(sharedStateRegistry);
+ verify(retrievableStateHandle1.retrieveState(),
times(1)).registerSharedStatesAfterRestored(sharedStateRegistry);
--- End diff --
The behaviour that is tested here is not strongly connected to the bug, I
just noticed it might be also worth checking to avoid future problems.
{{StandaloneCompletedCheckpointStore}} is not expected to do this, so there is
no need for a this particular test.
The question for a general test is valid. However, I noticed that
externalized checkpoints in general are not tested at all, so this is not just
a small addition but a whole new test (including some required test methods to
trigger and restore from externalized checkpoints in the JM) that would almost
justify a new planning task and PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
