You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "XComp (via GitHub)" <gi...@apache.org> on 2023/04/17 16:55:03 UTC

[GitHub] [flink] XComp commented on a diff in pull request #22380: [FLINK-31773][runtime] Separate DefaultLeaderElectionService.start(LeaderContender) into two separate methods for starting the driver and registering a contender

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


##########
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/JobMasterServiceLeadershipRunnerTest.java:
##########
@@ -772,7 +773,11 @@ void testJobMasterServiceLeadershipRunnerCloseWhenElectionServiceGrantLeaderShip
             closeAsyncCalledTrigger.await();
 
             final CheckedThread grantLeadershipThread =
-                    createCheckedThread(currentLeaderDriver::isLeader);
+                    createCheckedThread(
+                            () -> {
+                                currentLeaderDriver.notLeader();
+                                currentLeaderDriver.isLeader();

Review Comment:
   hmpf, I remember that I put it there because of some Precondition I have added to the `DefaultLeaderElectionService` to ensure that leadership can only be granted when the leadership wasn't acquired before. I'm pretty sure that it was related to the FLINK-31814 issue. :thinking: It's annoying that I cannot reproduce it anymore. Not sure what bug I had still in the `DefaultLeaderElectionService` which I tried to work around with this change. Anyway, you're right: This change is not necessary. I'm gonna revert it.



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