(solr) branch main updated: SOLR-17261: Allow cores to load without a timeout (#2431)

[ho...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

houston pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new f46812082d5 SOLR-17261: Allow cores to load without a timeout (#2431)
f46812082d5 is described below

commit f46812082d588da5cee05eaed8256845bcf7ffcd
Author: Houston Putman <ho...@apache.org>
AuthorDate: Wed May 1 21:05:44 2024 -0400

    SOLR-17261: Allow cores to load without a timeout (#2431)
    
    ExecutorUtil once again has the ability to wait
    for an ExecutorService to terminate without a timeout.
---
 solr/CHANGES.txt                                   |  2 +
 .../java/org/apache/solr/core/CoreContainer.java   |  4 +-
 .../org/apache/solr/common/util/ExecutorUtil.java  | 51 ++++++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index c2104977d57..5d04aa41ee8 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -115,6 +115,8 @@ Bug Fixes
 ---------------------
 * SOLR-12813: subqueries should respect basic auth. (Rudy Seitz via Eric Pugh)
 
+* SOLR-17261: Remove unintended timeout of 60 seconds for core loading. (Houston Putman)
+
 Dependency Upgrades
 ---------------------
 (No changes)
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index 8f3ee608011..b0489450f86 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1081,9 +1081,9 @@ public class CoreContainer {
     } finally {
       if (asyncSolrCoreLoad) {
         coreContainerWorkExecutor.submit(
-            () -> ExecutorUtil.shutdownAndAwaitTermination(coreLoadExecutor));
+            () -> ExecutorUtil.shutdownAndAwaitTerminationForever(coreLoadExecutor));
       } else {
-        ExecutorUtil.shutdownAndAwaitTermination(coreLoadExecutor);
+        ExecutorUtil.shutdownAndAwaitTerminationForever(coreLoadExecutor);
       }
     }
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java b/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
index b5bd42257d7..0fb3f8f4153 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/ExecutorUtil.java
@@ -106,18 +106,48 @@ public class ExecutorUtil {
     }
   }
 
+  /**
+   * Shutdown the {@link ExecutorService} and wait for 60 seconds for the threads to complete. More
+   * detail on the waiting can be found in {@link #awaitTermination(ExecutorService)}.
+   *
+   * @param pool The ExecutorService to shutdown and wait on
+   */
   public static void shutdownAndAwaitTermination(ExecutorService pool) {
     if (pool == null) return;
     pool.shutdown(); // Disable new tasks from being submitted
     awaitTermination(pool);
   }
 
+  /**
+   * Shutdown the {@link ExecutorService} and wait forever for the threads to complete. More detail
+   * on the waiting can be found in {@link #awaitTerminationForever(ExecutorService)}.
+   *
+   * <p>This should likely not be used in {@code close()} methods, as we want to timebound when
+   * shutting down. However, sometimes {@link ExecutorService}s are used to submit a list of tasks
+   * and awaiting termination is akin to waiting on the list of {@link Future}s to complete. In that
+   * case, this method should be used as there is no inherent time bound to waiting on those tasks
+   * to complete.
+   *
+   * @param pool The ExecutorService to shutdown and wait on
+   */
+  public static void shutdownAndAwaitTerminationForever(ExecutorService pool) {
+    if (pool == null) return;
+    pool.shutdown(); // Disable new tasks from being submitted
+    awaitTerminationForever(pool);
+  }
+
   public static void shutdownNowAndAwaitTermination(ExecutorService pool) {
     if (pool == null) return;
     pool.shutdownNow(); // Disable new tasks from being submitted; interrupt existing tasks
     awaitTermination(pool);
   }
 
+  /**
+   * Await the termination of an {@link ExecutorService} for a default of 60 seconds, then force
+   * shutdown the remaining threads and wait another 60 seconds.
+   *
+   * @param pool the ExecutorService to wait on
+   */
   public static void awaitTermination(ExecutorService pool) {
     awaitTermination(pool, 60, TimeUnit.SECONDS);
   }
@@ -143,6 +173,27 @@ public class ExecutorUtil {
     }
   }
 
+  /**
+   * Await the termination of an {@link ExecutorService} until all threads are complete, or until we
+   * are interrupted, at which point the {@link ExecutorService} will be interrupted as well.
+   *
+   * @param pool the ExecutorService to wait on
+   */
+  public static void awaitTerminationForever(ExecutorService pool) {
+    boolean shutdown = false;
+    try {
+      while (!shutdown) {
+        // Wait a while for existing tasks to terminate
+        shutdown = pool.awaitTermination(60, TimeUnit.SECONDS);
+      }
+    } catch (InterruptedException e) {
+      // Force cancel if current thread also interrupted
+      pool.shutdownNow();
+      // Preserve interrupt status
+      Thread.currentThread().interrupt();
+    }
+  }
+
   /** See {@link java.util.concurrent.Executors#newFixedThreadPool(int, ThreadFactory)} */
   public static ExecutorService newMDCAwareFixedThreadPool(
       int nThreads, ThreadFactory threadFactory) {