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 20:05:33 UTC
[solr] branch branch_9x updated (f91a8ba3cf4 -> dbd969daeb6)
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a change to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
from f91a8ba3cf4 logging, MDC: collection for action=DELETE (#1869)
new c2a2486aa73 tidy
new dbd969daeb6 SOLR-16879: throttle BACKUPCORE, RESTORECORE, SPLIT (#1864)
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
solr/CHANGES.txt | 4 +
.../OverseerCollectionMessageHandler.java | 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 ++
10 files changed, 121 insertions(+), 38 deletions(-)
[solr] 01/02: tidy
Posted by ds...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
commit c2a2486aa7309538379339abcf590e80c7715010
Author: David Smiley <ds...@salesforce.com>
AuthorDate: Thu Aug 31 16:04:58 2023 -0400
tidy
---
.../solr/cloud/api/collections/OverseerCollectionMessageHandler.java | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
index bd59bf8ede6..5597841d115 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerCollectionMessageHandler.java
@@ -114,7 +114,9 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler,
public OverseerSolrResponse processMessage(ZkNodeProps message, String operation) {
// sometimes overseer messages have the collection name in 'name' field, not 'collection'
MDCLoggingContext.setCollection(
- message.getStr(COLLECTION_PROP) != null ? message.getStr(COLLECTION_PROP) : message.getStr(NAME));
+ message.getStr(COLLECTION_PROP) != null
+ ? message.getStr(COLLECTION_PROP)
+ : message.getStr(NAME));
MDCLoggingContext.setShard(message.getStr(SHARD_ID_PROP));
MDCLoggingContext.setReplica(message.getStr(REPLICA_PROP));
log.debug("OverseerCollectionMessageHandler.processMessage : {} , {}", operation, message);
[solr] 02/02: SOLR-16879: throttle BACKUPCORE, RESTORECORE, SPLIT (#1864)
Posted by ds...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
commit dbd969daeb6061977f6274400ee76ae228a2adf9
Author: Pierre Salagnac <82...@users.noreply.github.com>
AuthorDate: Thu Aug 31 15:46:11 2023 -0400
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 f81bfc4aeac..bb00510a381 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -45,6 +45,10 @@ Improvements
* SOLR-15474: Make Circuit breakers individually pluggable (Atri Sharma, Christine Poerschke, janhoy)
+* 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)