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, one could
consider the `onRevokeLeadership` call to be 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.
Calling `onRevokeLeadership` is still useful if we want to losen the
code-coupling: Currently, the `LeaderElectionService` implementations assume
that the owner of `LeaderElectionService` is the `LeaderContender`. This is
true in the current code but this invariant doesn't need to be true necessarily
based on the interface.
--
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]
