[GitHub] [flink] pnowojski commented on a change in pull request #9885: [FLINK-14344][checkpointing] A preparation for snapshotting master hook state asynchronously

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

pnowojski commented on a change in pull request #9885: [FLINK-14344][checkpointing] A preparation for snapshotting master hook state asynchronously
URL: https://github.com/apache/flink/pull/9885#discussion_r343529308
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/checkpoint/CheckpointCoordinator.java
 ##########
 @@ -659,10 +660,10 @@ else if (!props.forceCheckpoint()) {
 				}
 
 				// trigger the master hooks for the checkpoint
-				final List<MasterState> masterStates = MasterHooks.triggerMasterHooks(masterHooks.values(),
+				final Map<String, MasterState> masterStates = MasterHooks.triggerMasterHooks(masterHooks.values(),
 
 Review comment:
   Ok. I think it still would be better to have this commit move to the PR that actually triggers master hooks asynchronously, but if this cause some non trivial conflicts (and also because I've already reviewed it ;) ), we can leave it here.
   
   But please expand the commit message explaining the intention. For example:
   ```
   [FLINK-14344][checkpointing] PendingCheckpoint supports acknowledging master state
   
   MasterState acknowledgements are now independent of task states acknowledgments. 
   This is done as a preparation for asynchronous MasterHooks firing, where we will first
   asynchronously fire MasterHooks, wait for their acknowledgements and only then start
   the checkpoint.
   ```

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


With regards,
Apache Git Services