zentol commented on code in PR #22601:
URL: https://github.com/apache/flink/pull/22601#discussion_r1221678062
##########
flink-runtime/src/main/java/org/apache/flink/runtime/leaderelection/AbstractLeaderElectionService.java:
##########
@@ -27,29 +27,37 @@
public abstract class AbstractLeaderElectionService implements
LeaderElectionService {
@Override
- public LeaderElection createLeaderElection() {
- return new DefaultLeaderElection(this);
+ public LeaderElection createLeaderElection(String contenderID) {
+ return new DefaultLeaderElection(this, contenderID);
}
/**
- * Registers the given {@link LeaderContender} with the underlying {@code
- * LeaderElectionService}. Leadership changes are starting to be reported
to the {@code
- * LeaderContender}.
+ * Registers the given {@link LeaderContender} under the passed {@code
contenderID} with the
+ * underlying {@code LeaderElectionService}. Leadership changes are
starting to be reported to
+ * the {@code LeaderContender}.
*/
- protected abstract void register(LeaderContender contender) throws
Exception;
+ protected abstract void register(String contenderID, LeaderContender
contender)
+ throws Exception;
- /** Removes the passed {@code LeaderContender} from the {@code
LeaderElectionService}. */
- protected abstract void remove(LeaderContender contender);
+ /**
+ * Removes the {@code LeaderContender} from the {@code
LeaderElectionService} that is associated
+ * with the passed {@code contenderID}.
Review Comment:
inconsistent uses of "given" vs "passed".
##########
flink-runtime/src/main/java/org/apache/flink/runtime/leaderelection/DefaultLeaderElection.java:
##########
@@ -31,36 +29,31 @@
class DefaultLeaderElection implements LeaderElection {
private final AbstractLeaderElectionService parentService;
- @Nullable private LeaderContender leaderContender;
+ private String contenderID;
Review Comment:
final
##########
flink-runtime/src/main/java/org/apache/flink/runtime/highavailability/AbstractHaServices.java:
##########
@@ -247,7 +247,7 @@ private LeaderElection createLeaderElection(String
leaderName) throws Exception
leaderElectionService.startLeaderElectionBackend();
closeableRegistry.registerCloseable(leaderElectionService);
- return leaderElectionService.createLeaderElection();
+ return leaderElectionService.createLeaderElection(leaderName);
Review Comment:
Isn't this actually a bug? How does this work with the multi component LES?
##########
flink-runtime/src/main/java/org/apache/flink/runtime/leaderelection/AbstractLeaderElectionService.java:
##########
@@ -27,29 +27,37 @@
public abstract class AbstractLeaderElectionService implements
LeaderElectionService {
@Override
- public LeaderElection createLeaderElection() {
- return new DefaultLeaderElection(this);
+ public LeaderElection createLeaderElection(String contenderID) {
+ return new DefaultLeaderElection(this, contenderID);
}
/**
- * Registers the given {@link LeaderContender} with the underlying {@code
- * LeaderElectionService}. Leadership changes are starting to be reported
to the {@code
- * LeaderContender}.
+ * Registers the given {@link LeaderContender} under the passed {@code
contenderID} with the
+ * underlying {@code LeaderElectionService}. Leadership changes are
starting to be reported to
+ * the {@code LeaderContender}.
*/
- protected abstract void register(LeaderContender contender) throws
Exception;
+ protected abstract void register(String contenderID, LeaderContender
contender)
+ throws Exception;
- /** Removes the passed {@code LeaderContender} from the {@code
LeaderElectionService}. */
- protected abstract void remove(LeaderContender contender);
+ /**
+ * Removes the {@code LeaderContender} from the {@code
LeaderElectionService} that is associated
+ * with the passed {@code contenderID}.
+ */
+ protected abstract void remove(String contenderID);
- /** Confirms the leadership with the given session ID and address. */
- protected abstract void confirmLeadership(UUID leaderSessionID, String
leaderAddress);
+ /**
+ * Confirms the leadership with the given session ID and address for the
passed {@code
+ * contenderID}.
Review Comment:
"Confirms the leadership for the given contender under the given session ..."
##########
flink-runtime/src/main/java/org/apache/flink/runtime/highavailability/AbstractHaServices.java:
##########
@@ -247,7 +247,7 @@ private LeaderElection createLeaderElection(String
leaderName) throws Exception
leaderElectionService.startLeaderElectionBackend();
closeableRegistry.registerCloseable(leaderElectionService);
- return leaderElectionService.createLeaderElection();
+ return leaderElectionService.createLeaderElection(leaderName);
Review Comment:
hmm. This isn't really a contender Id (which would identify a single
contender), but rather the contender _type/role_ isn't it? These are values
like "restserver", "dispatcher" etc..
Like, you can have 2 jobmanager _contenders_ contending for the _role_ of
the leading jobmanager.
--
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]
