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