wangyang0918 commented on code in PR #24132:
URL: https://github.com/apache/flink/pull/24132#discussion_r1467160375
##########
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/resources/KubernetesLeaderElectorITCase.java:
##########
@@ -110,4 +114,67 @@ void testMultipleKubernetesLeaderElectors() throws
Exception {
kubernetesExtension.getFlinkKubeClient().deleteConfigMap(leaderConfigMapName).get();
}
}
+
+ /**
+ * This test verifies that the {@link KubernetesLeaderElector} is able to
handle scenario where
+ * the lease cannot be renewed.
+ *
+ * <p>See FLINK-34007 for further details.
+ */
+ @Test
+ void testLeaderElectorLifecycleManagement() throws Exception {
Review Comment:
Great test. We might need to delete the leader elector ConfigMap because
fabricio {{LeaderElector}} does not clean up.
##########
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/resources/KubernetesLeaderElector.java:
##########
@@ -86,12 +117,38 @@ public KubernetesLeaderElector(
newLeader,
leaderConfig.getConfigMapName())))
.build();
+ this.executorService = executorService;
+
+ LOG.info(
+ "Create KubernetesLeaderElector on lock {}.",
+ leaderElectionConfig.getLock().describe());
+ }
+
+ @GuardedBy("lock")
+ private void resetInternalLeaderElector() {
+ stopLeaderElectionCycle();
+
internalLeaderElector =
new LeaderElector(kubernetesClient, leaderElectionConfig,
executorService);
+ currentLeaderElectionSession = internalLeaderElector.start();
+
LOG.info(
- "Create KubernetesLeaderElector {} with lock identity {}.",
- leaderConfig.getConfigMapName(),
- leaderConfig.getLockIdentity());
+ "Triggered leader election on lock {}.",
leaderElectionConfig.getLock().describe());
+ }
+
+ @GuardedBy("lock")
+ private void stopLeaderElectionCycle() {
+ if (internalLeaderElector != null) {
+ Preconditions.checkNotNull(currentLeaderElectionSession);
+
+ // the current leader election cycle needs to be cancelled before
releasing the lock to
+ // avoid retrying
+ currentLeaderElectionSession.cancel(true);
+ currentLeaderElectionSession = null;
+
+ internalLeaderElector.release();
Review Comment:
Executing the `LeaderElector#release()` will trigger another `notLeader()`
callback, which revoke the leader contender leadership again. It might be
harmless because the leader contender already lost leadership.
Could you please share me why we do not simply add
`.withReleaseOnCancel(true)` when building the `leaderElectionConfig`?
--
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]
