zentol commented on code in PR #22769:
URL: https://github.com/apache/flink/pull/22769#discussion_r1229182371
##########
flink-runtime/src/main/java/org/apache/flink/runtime/leaderelection/DefaultLeaderElectionService.java:
##########
@@ -206,34 +223,35 @@ protected final void remove(LeaderContender contender) {
@Override
public void close() throws Exception {
+ Preconditions.checkState(
+ leaderElectionDriver != null, "The HA backend wasn't
initialized.");
+
synchronized (lock) {
Preconditions.checkState(
leaderContender == null,
"The DefaultLeaderElectionService should have been stopped
before closing the instance.");
issuedLeaderSessionID = null;
- if (leaderElectionDriver != null) {
- leaderElectionDriver.close();
- leaderElectionDriver = null;
-
- // The shutdown of the thread pool needs to be done forcefully
because we want its
- // lifecycle being coupled to the driver (which require it to
be shut down within
- // the lock) to allow null checks in runInLeaderEventThread
method. The outstanding
- // event handling callbacks are going to be ignored, anyway.
- final List<Runnable> outstandingEventHandlingCalls =
-
Preconditions.checkNotNull(leadershipOperationExecutor).shutdownNow();
- if (!outstandingEventHandlingCalls.isEmpty()) {
- LOG.debug(
- "The DefaultLeaderElectionService was closed with
{} still not being processed. No further action necessary.",
- outstandingEventHandlingCalls.size() == 1
- ? "one event"
- : (outstandingEventHandlingCalls.size() +
" events"));
- }
+ if (running) {
+ running = false;
} else {
LOG.debug("The HA backend connection isn't established. No
actions taken.");
}
}
+
+ leaderElectionDriver.close();
+
+ // interrupt any outstanding events
+ final List<Runnable> outstandingEventHandlingCalls =
+
Preconditions.checkNotNull(leadershipOperationExecutor).shutdownNow();
+ if (!outstandingEventHandlingCalls.isEmpty()) {
+ LOG.debug(
+ "The DefaultLeaderElectionService was closed with {} still
not being processed. No further action necessary.",
+ outstandingEventHandlingCalls.size() == 1
+ ? "one event"
+ : (outstandingEventHandlingCalls.size() + "
events"));
Review Comment:
```suggestion
"The DefaultLeaderElectionService was closed with {}
event(s) still not being processed. No further action necessary.",
outstandingEventHandlingCalls.size());
```
nit: Just seems overly complicated,
##########
flink-runtime/src/main/java/org/apache/flink/runtime/leaderelection/DefaultLeaderElectionService.java:
##########
@@ -206,34 +223,35 @@ protected final void remove(LeaderContender contender) {
@Override
public void close() throws Exception {
+ Preconditions.checkState(
+ leaderElectionDriver != null, "The HA backend wasn't
initialized.");
Review Comment:
shouldnt this run under the lock?
--
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]
