[GitHub] [flink] XComp commented on a diff in pull request #21742: [FLINK-30765][runtime] Aligns the LeaderElectionService.stop() contract

["XComp (via GitHub)" <gi...@apache.org>]

XComp commented on code in PR #21742:
URL: https://github.com/apache/flink/pull/21742#discussion_r1082832944


##########
flink-runtime/src/main/java/org/apache/flink/runtime/leaderelection/DefaultLeaderElectionService.java:
##########
@@ -100,6 +100,7 @@ public final void stop() throws Exception {
             if (!running) {
                 return;
             }
+            leaderContender.revokeLeadership();

Review Comment:
   Ok, digging into a the PR a bit more shows that the old [ZooKeeperLeaderElectionService](https://github.com/apache/flink/pull/13644/files?show-deleted-files=true&show-viewed-files=true&file-filters%5B%5D=#diff-de2fa2be46667dad62b57be6fefa6135043e19888afd453b634792241785c8e0L147) didn't call the revoke. So, I guess, that's just keeping the implementation like that.
   
   But just in general from a theoretical standpoint: We do want to consistently call revoke during shutdown to trigger the clean up through revokeLeadership to stick to the right protocol. That, as a consequence, would make cleanup in the `LeaderContender.stop()` implementation obsolete. WDYT? :thinking: 



-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org