anverus commented on a change in pull request #1997:
URL: https://github.com/apache/lucene-solr/pull/1997#discussion_r506736783



##########
File path: solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java
##########
@@ -1372,55 +1372,55 @@ private boolean isEnabled( 
@SuppressWarnings({"rawtypes"})NamedList params ){
     return Boolean.TRUE.equals( enable );
   }
 
-  /**
-   * register a closehook
-   */
-  private void registerCloseHook() {
-    core.addCloseHook(new CloseHook() {
-      @Override
-      public void preClose(SolrCore core) {
-        if (executorService != null) executorService.shutdown(); // we don't 
wait for shutdown - this can deadlock core reload
-      }
+  private final CloseHook startShutdownHook = new CloseHook() {
+    @Override
+    public void preClose(SolrCore core) {
+      if (executorService != null)
+        executorService.shutdown(); // we don't wait for shutdown - this can 
deadlock core reload
+    }
 
-      @Override
-      public void postClose(SolrCore core) {
-        if (pollingIndexFetcher != null) {
-          pollingIndexFetcher.destroy();
-        }
-        if (currentIndexFetcher != null && currentIndexFetcher != 
pollingIndexFetcher) {
-          currentIndexFetcher.destroy();
-        }
+    @Override
+    public void postClose(SolrCore core) {
+      if (pollingIndexFetcher != null) {
+        pollingIndexFetcher.destroy();
       }
-    });
-
-    core.addCloseHook(new CloseHook() {
-      @Override
-      public void preClose(SolrCore core) {
-        ExecutorUtil.shutdownAndAwaitTermination(restoreExecutor);
-        if (restoreFuture != null) {
-          restoreFuture.cancel(false);
-        }
+      if (currentIndexFetcher != null && currentIndexFetcher != 
pollingIndexFetcher) {
+        currentIndexFetcher.destroy();
       }
+    }
+  };
+  private final CloseHook finishShutdownHook = new CloseHook() {
+    @Override
+    public void preClose(SolrCore core) {
+      ExecutorUtil.shutdownAndAwaitTermination(restoreExecutor);
+      if (restoreFuture != null) {
+        restoreFuture.cancel(false);
+      }
+    }
 
-      @Override
-      public void postClose(SolrCore core) {}
-    });
+    @Override
+    public void postClose(SolrCore core) {
+    }
+  };
+
+  /**
+   * register a closehook
+   */
+  private void registerCloseHook() {
+    core.addCloseHook(startShutdownHook);
+    core.addCloseHook(finishShutdownHook);
   }
 
   public void shutdown() {
-    if (executorService != null) executorService.shutdown();
-    if (pollingIndexFetcher != null) {
-      pollingIndexFetcher.destroy();
-    }
-    if (currentIndexFetcher != null && currentIndexFetcher != 
pollingIndexFetcher) {
-      currentIndexFetcher.destroy();
-    }
-    ExecutorUtil.shutdownAndAwaitTermination(restoreExecutor);
-    if (restoreFuture != null) {
-      restoreFuture.cancel(false);
-    }
-    
+    startShutdownHook.preClose(core);
+    startShutdownHook.postClose(core);
+    finishShutdownHook.preClose(core);
+    finishShutdownHook.postClose(core);

Review comment:
       Order of these calls copies order of execution of instructions in 
original shutdown method, while the correct order seems to be 
   ```
       startShutdownHook.preClose(core);
       finishShutdownHook.preClose(core);
       startShutdownHook.postClose(core);
       finishShutdownHook.postClose(core);
   ```




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to