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


##########
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:
   I re-iterated over that issue once more and realized that I looked at it from the wrong ankle: There's no reason to call `onRevokeLeadership` as part of the `LeaderElectionService.stop()` call. `stop()` is always called by the contender as part of its shutdown process. Therefore, the contender is already aware of shutting down the leader election process. That's why, we can consider the `onRevokeLeadership` call as obsolete. The `onRevokeLeadership` call would be ignored, anyway, because of the `running` flag that is set to `false` in any `LeaderContender` implementation before shutting down the `LeaderElectionService`. That flag is checked in the `onRevokeLeadership` implementations and would omit any logic. Working around this would require a bigger refactoring and is not necessary.
   
   Therefore, instead of adding the `onRevokeLeadership` to the shutdown procedure we can remove this call from the `LeaderElectionService` implementations that do call it (also including the `MultipleComponentLeaderElectionService` which does call it without any necessity).
   
   I'm gonna update the PR to consider this.



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