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


##########
flink-runtime/src/main/java/org/apache/flink/runtime/leaderelection/DefaultLeaderElectionService.java:
##########
@@ -32,49 +31,73 @@
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.GuardedBy;
 
+import java.util.List;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
 import static org.apache.flink.util.Preconditions.checkNotNull;
 
 /**
  * Default implementation for leader election service. Composed with different 
{@link
  * LeaderElectionDriver}, we could perform a leader election for the 
contender, and then persist the
  * leader information to various storage.
+ *
+ * <p>{@code DefaultLeaderElectionService} handles a single {@link 
LeaderContender}.
  */
 public class DefaultLeaderElectionService
-        implements LeaderElectionService, LeaderElectionEventHandler {
+        implements LeaderElectionService, LeaderElectionEventHandler, 
AutoCloseable {
 
     private static final Logger LOG = 
LoggerFactory.getLogger(DefaultLeaderElectionService.class);
 
     private final Object lock = new Object();
 
     private final LeaderElectionDriverFactory leaderElectionDriverFactory;
 
-    /** The leader contender which applies for leadership. */
+    /**
+     * {@code leaderContender} being {@code null} indicates that no {@link 
LeaderContender} is
+     * registered that participates in the leader election, yet. See {@link 
#start(LeaderContender)}
+     * and {@link #stop()} for lifecycle management.
+     *
+     * <p>{@code @Nullable} isn't used here to avoid having multiple warnings 
spread over this class
+     * in a supporting IDE.
+     */
     @GuardedBy("lock")
-    // @Nullable is commented-out to avoid having multiple warnings spread 
over this class
-    // this.running=true ensures that leaderContender != null
-    private volatile LeaderContender leaderContender;
+    private LeaderContender leaderContender;
 
+    /**
+     * Saves the session ID which was issued by the {@link 
LeaderElectionDriver} if and only if the
+     * leadership is acquired by this service. {@code issuedLeaderSessionID} 
being {@code null}
+     * indicates that this service isn't the leader right now (i.e. {@link
+     * #onGrantLeadership(UUID)}) wasn't called, yet (independently of what 
{@code
+     * leaderElectionDriver#hasLeadership()} returns).
+     */
     @GuardedBy("lock")
     @Nullable
-    private volatile UUID issuedLeaderSessionID;
+    private UUID issuedLeaderSessionID;
 
+    /**
+     * Saves the leader information for a registered {@link LeaderContender} 
after this contender
+     * confirmed the leadership.
+     */
     @GuardedBy("lock")
-    private volatile LeaderInformation confirmedLeaderInformation;
+    private LeaderInformation confirmedLeaderInformation;
 
+    /**
+     * {@code leaderElectionDriver} being {@code null} indicates that the 
connection to the
+     * LeaderElection backend isn't established, yet. See {@link 
#startLeaderElectionBackend()} and
+     * {@link #close()} for lifecycle management. The lifecycle of the driver 
should have been
+     * established before registering a {@link LeaderContender} and stopped 
after the contender has
+     * been removed.
+     *
+     * <p>{@code @Nullable} isn't used here to avoid having multiple warnings 
spread over this class
+     * in a supporting IDE.
+     */
     @GuardedBy("lock")
-    private volatile boolean running;
+    private volatile LeaderElectionDriver leaderElectionDriver;
 
-    // @Nullable is commented-out to avoid having multiple warnings spread 
over this class
-    // this.running=true ensures that leaderContender != null
-    private LeaderElectionDriver leaderElectionDriver;
-
-    private final ExecutorService leadershipOperationExecutor;
+    @Nullable private ExecutorService leadershipOperationExecutor;

Review Comment:
   args, good idea - I should have gone through the `ExecutorService` interface 
for such a check.



-- 
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-unsubscr...@flink.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to