wangyang0918 commented on code in PR #21742:
URL: https://github.com/apache/flink/pull/21742#discussion_r1099582674
##########
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:
Sorry for the late response.
I believe that you are right we have some redundant codes in
`close/closeAsync()` and `revokeLeadership()` of all `LeaderContender`
implementations. And I get your point why you want to call the
`revokeLeadership` in the shutdown process.
However, I strongly have a mind that the leader information stored in the
ZNode and ConfigMap needs to be cleaned up if a leader contender is revoked
leadership. Before this change, this is true since the `revokeLeadership`
happens along with `clearConfirmedLeaderInformation`.
Moreover, from the point of view of leader elector, I think it does not lose
leadership when stopping the contender since the timeout has not yet expired.
So I lean to not explicitly call the `leaderContender.revokeLeadership()`
here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]