[GitHub] [zookeeper] lvfangmin commented on a change in pull request #1130: [ZOOKEEPER-3598] Fix potential data inconsistency issue due to CommitProcessor not gracefully shutdown

[GitBox <gi...@apache.org>]

lvfangmin commented on a change in pull request #1130: [ZOOKEEPER-3598] Fix potential data inconsistency issue due to CommitProcessor not gracefully shutdown
URL: https://github.com/apache/zookeeper/pull/1130#discussion_r342704229
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
 ##########
 @@ -621,6 +622,20 @@ public void shutdown() {
             workerPool.join(workerShutdownTimeoutMS);
         }
 
+        try {
+            this.join(workerShutdownTimeoutMS);
+        } catch (InterruptedException e) {
+            LOG.warn("Interrupted while waiting for CommitProcessor to finish");
+            Thread.currentThread().interrupt();
+        }
+
+        if (this.isAlive()) {
+            LOG.warn("CommitProcessor does not shutdown gracefully after "
+                    + "waiting for {} ms, exit to avoid potential "
+                    + "inconsistency issue", workerShutdownTimeoutMS);
+            System.exit(ExitCode.SHUTDOWN_UNGRACEFULLY.getValue());
 
 Review comment:
   @eolivelli we still try to halt before checking and exit here, but in case the CommitProcessor thread itself cannot shutdown within given time, we need to quit safely. And as @hanm mentioned we don't use shutdown hook here.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services