You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ds...@apache.org on 2023/08/31 19:46:17 UTC

[solr] branch main updated: SOLR-16879: throttle BACKUPCORE, RESTORECORE, SPLIT (#1864)

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

dsmiley 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 585ba9ba5f9 SOLR-16879: throttle BACKUPCORE, RESTORECORE, SPLIT (#1864)
585ba9ba5f9 is described below

commit 585ba9ba5f9afdaf00ff2071504c4b62f2234d1f
Author: Pierre Salagnac <82...@users.noreply.github.com>
AuthorDate: Thu Aug 31 21:46:11 2023 +0200

    SOLR-16879: throttle BACKUPCORE, RESTORECORE, SPLIT (#1864)
    
    Core Admin operations that are submitted with "async" that are also considered expensive will use a different thread pool of a lower size.  Expensive tasks are: BACKUPCORE, RESTORECORE, SPLIT
    
    
    Co-authored-by: Pierre Salagnac <ps...@salesforce.com>
    Co-authored-by: David Smiley <ds...@salesforce.com>
---
 solr/CHANGES.txt                                   |  4 +
 .../apache/solr/handler/admin/BackupCoreOp.java    |  5 ++
 .../solr/handler/admin/CoreAdminHandler.java       | 97 +++++++++++++++-------
 .../solr/handler/admin/CoreAdminOperation.java     |  6 ++
 .../apache/solr/handler/admin/RestoreCoreOp.java   |  6 ++
 .../org/apache/solr/handler/admin/SplitOp.java     |  5 ++
 .../solr/handler/admin/api/BackupCoreAPI.java      |  5 ++
 .../solr/handler/admin/api/CoreAdminAPIBase.java   | 22 +++--
 .../solr/handler/admin/api/RestoreCoreAPI.java     |  5 ++
 9 files changed, 118 insertions(+), 37 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index ce74fc3b1cb..e7e31b686d5 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -107,6 +107,10 @@ Improvements
 
 * SOLR-16927: Allow SolrClientCache clients to use Jetty HTTP2 clients (Alex Deparvu, David Smiley)
 
+* SOLR-16879: Limit the number of concurrent expensive core admin operations by running them in a
+  dedicated thread pool. Backup, Restore and Split are expensive operations.
+  (Pierre Salagnac, David Smiley)
+
 Optimizations
 ---------------------
 
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java b/solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java
index bfd939c43b4..788fb494757 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/BackupCoreOp.java
@@ -28,6 +28,11 @@ import org.apache.solr.handler.api.V2ApiUtils;
 
 class BackupCoreOp implements CoreAdminHandler.CoreAdminOp {
 
+  @Override
+  public boolean isExpensive() {
+    return true;
+  }
+
   @Override
   public void execute(CoreAdminHandler.CallInfo it) throws Exception {
     final SolrParams params = it.req.getParams();
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
index 94212c883cf..f9819dcaf80 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java
@@ -97,6 +97,7 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
   protected final CoreAdminAsyncTracker coreAdminAsyncTracker;
 
   public static String RESPONSE_STATUS = "STATUS";
+
   public static String RESPONSE_MESSAGE = "msg";
   public static String OPERATION_RESPONSE = "response";
 
@@ -129,13 +130,21 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
   @Override
   public void initializeMetrics(SolrMetricsContext parentContext, String scope) {
     super.initializeMetrics(parentContext, scope);
-    coreAdminAsyncTracker.parallelExecutor =
+    coreAdminAsyncTracker.standardExecutor =
         MetricUtils.instrumentedExecutorService(
-            coreAdminAsyncTracker.parallelExecutor,
+            coreAdminAsyncTracker.standardExecutor,
             this,
             solrMetricsContext.getMetricRegistry(),
             SolrMetricManager.mkName(
                 "parallelCoreAdminExecutor", getCategory().name(), scope, "threadPool"));
+
+    coreAdminAsyncTracker.expensiveExecutor =
+        MetricUtils.instrumentedExecutorService(
+            coreAdminAsyncTracker.expensiveExecutor,
+            this,
+            solrMetricsContext.getMetricRegistry(),
+            SolrMetricManager.mkName(
+                "parallelCoreExpensiveAdminExecutor", getCategory().name(), scope, "threadPool"));
   }
 
   @Override
@@ -196,8 +205,6 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
       }
       // boolean doPersist = false;
       final String taskId = req.getParams().get(CommonAdminParams.ASYNC);
-      final CoreAdminAsyncTracker.TaskObject taskObject =
-          new CoreAdminAsyncTracker.TaskObject(taskId);
 
       // Pick the action
       final String action = req.getParams().get(ACTION, STATUS.toString()).toLowerCase(Locale.ROOT);
@@ -220,13 +227,16 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
       if (taskId == null) {
         callInfo.call();
       } else {
-        coreAdminAsyncTracker.submitAsyncTask(
-            taskObject,
-            action,
+        Callable<SolrQueryResponse> task =
             () -> {
               callInfo.call();
               return callInfo.rsp;
-            });
+            };
+
+        var taskObject =
+            new CoreAdminAsyncTracker.TaskObject(taskId, action, op.isExpensive(), task);
+
+        coreAdminAsyncTracker.submitAsyncTask(taskObject);
       }
     } finally {
       rsp.setHttpCaching(false);
@@ -332,7 +342,7 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
 
   /** Method to ensure shutting down of the ThreadPool Executor. */
   public void shutdown() {
-    if (coreAdminAsyncTracker.parallelExecutor != null) coreAdminAsyncTracker.shutdown();
+    coreAdminAsyncTracker.shutdown();
   }
 
   private static final Map<String, CoreAdminOp> opMap = new HashMap<>();
@@ -393,6 +403,11 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
   }
 
   public interface CoreAdminOp {
+
+    default boolean isExpensive() {
+      return false;
+    }
+
     /**
      * @param it request/response object
      *     <p>If the request is invalid throw a SolrException with
@@ -411,10 +426,18 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
     public static final String FAILED = "failed";
     public final Map<String, Map<String, TaskObject>> requestStatusMap;
 
-    private ExecutorService parallelExecutor =
+    // Executor for all standard tasks (the ones that are not flagged as expensive)
+    // We always keep 50 live threads
+    private ExecutorService standardExecutor =
         ExecutorUtil.newMDCAwareFixedThreadPool(
             50, new SolrNamedThreadFactory("parallelCoreAdminAPIBaseExecutor"));
 
+    // Executor for expensive tasks
+    // We keep the number number of max threads very low to have throttling for expensive tasks
+    private ExecutorService expensiveExecutor =
+        ExecutorUtil.newMDCAwareCachedThreadPool(
+            5, new SolrNamedThreadFactory("parallelCoreAdminAPIExpensiveExecutor"));
+
     public CoreAdminAsyncTracker() {
       HashMap<String, Map<String, TaskObject>> map = new HashMap<>(3, 1.0f);
       map.put(RUNNING, Collections.synchronizedMap(new LinkedHashMap<>()));
@@ -424,36 +447,41 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
     }
 
     public void shutdown() {
-      ExecutorUtil.shutdownAndAwaitTermination(parallelExecutor);
+      ExecutorUtil.shutdownAndAwaitTermination(standardExecutor);
+      ExecutorUtil.shutdownAndAwaitTermination(expensiveExecutor);
     }
 
     public Map<String, TaskObject> getRequestStatusMap(String key) {
       return requestStatusMap.get(key);
     }
 
-    public void submitAsyncTask(
-        TaskObject taskObject, String action, Callable<SolrQueryResponse> task)
-        throws SolrException {
+    public void submitAsyncTask(TaskObject taskObject) throws SolrException {
       ensureTaskIdNotInUse(taskObject.taskId);
       addTask(RUNNING, taskObject);
 
+      Runnable command =
+          () -> {
+            boolean exceptionCaught = false;
+            try {
+              final SolrQueryResponse response = taskObject.task.call();
+              taskObject.setRspObject(response);
+              taskObject.setOperationRspObject(response);
+            } catch (Exception e) {
+              exceptionCaught = true;
+              taskObject.setRspObjectFromException(e);
+            } finally {
+              finishTask(taskObject, !exceptionCaught);
+            }
+          };
+
       try {
         MDC.put("CoreAdminHandler.asyncId", taskObject.taskId);
-        MDC.put("CoreAdminHandler.action", action);
-        parallelExecutor.execute(
-            () -> {
-              boolean exceptionCaught = false;
-              try {
-                final SolrQueryResponse response = task.call();
-                taskObject.setRspObject(response);
-                taskObject.setOperationRspObject(response);
-              } catch (Exception e) {
-                exceptionCaught = true;
-                taskObject.setRspObjectFromException(e);
-              } finally {
-                finishTask(taskObject, !exceptionCaught);
-              }
-            });
+        MDC.put("CoreAdminHandler.action", taskObject.action);
+        if (taskObject.expensive) {
+          expensiveExecutor.execute(command);
+        } else {
+          standardExecutor.execute(command);
+        }
       } finally {
         MDC.remove("CoreAdminHandler.asyncId");
         MDC.remove("CoreAdminHandler.action");
@@ -503,12 +531,19 @@ public class CoreAdminHandler extends RequestHandlerBase implements PermissionNa
      * response (if available).
      */
     public static class TaskObject {
-      public String taskId;
+      final String taskId;
+      final String action;
+      final boolean expensive;
+      final Callable<SolrQueryResponse> task;
       public String rspInfo;
       public Object operationRspInfo;
 
-      public TaskObject(String taskId) {
+      public TaskObject(
+          String taskId, String action, boolean expensive, Callable<SolrQueryResponse> task) {
         this.taskId = taskId;
+        this.action = action;
+        this.expensive = expensive;
+        this.task = task;
       }
 
       public String getRspObject() {
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
index bf82ae38c4e..31251759c76 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CoreAdminOperation.java
@@ -309,6 +309,12 @@ public enum CoreAdminOperation implements CoreAdminOp {
     return log;
   }
 
+  @Override
+  public boolean isExpensive() {
+    // delegates this to the actual implementation
+    return fun.isExpensive();
+  }
+
   /**
    * Returns the core status for a particular core.
    *
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java b/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java
index b7f97469b49..c95af84d3d7 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/RestoreCoreOp.java
@@ -24,6 +24,12 @@ import org.apache.solr.handler.admin.api.RestoreCoreAPI;
 import org.apache.solr.handler.api.V2ApiUtils;
 
 class RestoreCoreOp implements CoreAdminHandler.CoreAdminOp {
+
+  @Override
+  public boolean isExpensive() {
+    return true;
+  }
+
   @Override
   public void execute(CoreAdminHandler.CallInfo it) throws Exception {
     final SolrParams params = it.req.getParams();
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/SplitOp.java b/solr/core/src/java/org/apache/solr/handler/admin/SplitOp.java
index 3b66794c5d5..fd29d31fcd0 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/SplitOp.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/SplitOp.java
@@ -70,6 +70,11 @@ class SplitOp implements CoreAdminHandler.CoreAdminOp {
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
+  @Override
+  public boolean isExpensive() {
+    return true;
+  }
+
   @Override
   public void execute(CoreAdminHandler.CallInfo it) throws Exception {
     SolrParams params = it.req.getParams();
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/BackupCoreAPI.java b/solr/core/src/java/org/apache/solr/handler/admin/api/BackupCoreAPI.java
index 8f57c58f2d9..580f2849b2f 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/api/BackupCoreAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/api/BackupCoreAPI.java
@@ -56,6 +56,11 @@ public class BackupCoreAPI extends CoreAdminAPIBase {
     super(coreContainer, coreAdminAsyncTracker, solrQueryRequest, solrQueryResponse);
   }
 
+  @Override
+  boolean isExpensive() {
+    return true;
+  }
+
   @POST
   @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
   @PermissionName(COLL_EDIT_PERM)
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java b/solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java
index 6a4130e9132..69fe4c993ce 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/api/CoreAdminAPIBase.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.handler.admin.api;
 
+import java.util.concurrent.Callable;
 import java.util.function.Supplier;
 import org.apache.solr.api.JerseyResource;
 import org.apache.solr.client.api.model.SolrJerseyResponse;
@@ -52,6 +53,14 @@ public abstract class CoreAdminAPIBase extends JerseyResource {
     this.rsp = rsp;
   }
 
+  /**
+   * Can be overridden by operations that are expensive, so we don't execute too many of them
+   * concurrently.
+   */
+  boolean isExpensive() {
+    return false;
+  }
+
   /**
    * Wraps the subclasses logic with extra bookkeeping logic.
    *
@@ -79,22 +88,23 @@ public abstract class CoreAdminAPIBase extends JerseyResource {
         throw new SolrException(
             SolrException.ErrorCode.BAD_REQUEST, "Core container instance missing");
       }
-      final CoreAdminHandler.CoreAdminAsyncTracker.TaskObject taskObject =
-          new CoreAdminHandler.CoreAdminAsyncTracker.TaskObject(taskId);
 
       MDCLoggingContext.setCoreName(coreName);
       TraceUtils.setDbInstance(req, coreName);
       if (taskId == null) {
         return supplier.get();
       } else {
-        coreAdminAsyncTracker.submitAsyncTask(
-            taskObject,
-            actionName,
+        Callable<SolrQueryResponse> task =
             () -> {
               T response = supplier.get();
               V2ApiUtils.squashIntoSolrResponseWithoutHeader(rsp, response);
               return rsp;
-            });
+            };
+
+        final CoreAdminHandler.CoreAdminAsyncTracker.TaskObject taskObject =
+            new CoreAdminHandler.CoreAdminAsyncTracker.TaskObject(
+                taskId, actionName, isExpensive(), task);
+        coreAdminAsyncTracker.submitAsyncTask(taskObject);
       }
     } catch (CoreAdminAPIBaseException e) {
       throw e.trueException;
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCoreAPI.java b/solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCoreAPI.java
index 03358880450..7e8ccf29862 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCoreAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/api/RestoreCoreAPI.java
@@ -61,6 +61,11 @@ public class RestoreCoreAPI extends CoreAdminAPIBase {
     super(coreContainer, coreAdminAsyncTracker, solrQueryRequest, solrQueryResponse);
   }
 
+  @Override
+  boolean isExpensive() {
+    return true;
+  }
+
   @POST
   @Produces({"application/json", "application/xml", BINARY_CONTENT_TYPE_V2})
   @PermissionName(CORE_EDIT_PERM)