dsmiley commented on code in PR #1761: URL: https://github.com/apache/solr/pull/1761#discussion_r1255883240
########## solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java: ########## @@ -403,60 +410,106 @@ public interface CoreAdminOp { public static class CoreAdminAsyncTracker { private static final int MAX_TRACKED_REQUESTS = 100; + public static final String PENDING = "pending"; public static final String RUNNING = "running"; public static final String COMPLETED = "completed"; public static final String FAILED = "failed"; public final Map<String, Map<String, TaskObject>> requestStatusMap; + static final int MAX_CONCURRENT_EXPENSIVE_TASKS = + Integer.getInteger("solr.admin.maxConcurrentExpensiveTasks", 5); + private volatile boolean shutdown; + private int expensiveRunningTasks; + private final ConcurrentLinkedQueue<TaskObject> expensiveTaskQueue; Review Comment: Not actually used concurrently though ########## solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java: ########## @@ -218,13 +217,16 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw if (taskId == null) { callInfo.call(); } else { - coreAdminAsyncTracker.submitAsyncTask( - taskObject, - action, + Callable<SolrQueryResponse> task = () -> { callInfo.call(); return callInfo.rsp; - }); + }; + + final CoreAdminAsyncTracker.TaskObject taskObject = Review Comment: Just want to mention that Java 11 "var" keyword would be useful here where there is a very long type, and we know the type clearly here anyway. Or just inline this. ########## solr/core/src/java/org/apache/solr/handler/admin/CoreAdminHandler.java: ########## @@ -403,60 +410,106 @@ public interface CoreAdminOp { public static class CoreAdminAsyncTracker { private static final int MAX_TRACKED_REQUESTS = 100; + public static final String PENDING = "pending"; public static final String RUNNING = "running"; public static final String COMPLETED = "completed"; public static final String FAILED = "failed"; public final Map<String, Map<String, TaskObject>> requestStatusMap; + static final int MAX_CONCURRENT_EXPENSIVE_TASKS = + Integer.getInteger("solr.admin.maxConcurrentExpensiveTasks", 5); + private volatile boolean shutdown; + private int expensiveRunningTasks; + private final ConcurrentLinkedQueue<TaskObject> expensiveTaskQueue; + private ExecutorService parallelExecutor = ExecutorUtil.newMDCAwareFixedThreadPool( 50, new SolrNamedThreadFactory("parallelCoreAdminAPIBaseExecutor")); public CoreAdminAsyncTracker() { HashMap<String, Map<String, TaskObject>> map = new HashMap<>(3, 1.0f); + map.put(PENDING, Collections.synchronizedMap(new LinkedHashMap<>())); map.put(RUNNING, Collections.synchronizedMap(new LinkedHashMap<>())); map.put(COMPLETED, Collections.synchronizedMap(new LinkedHashMap<>())); map.put(FAILED, Collections.synchronizedMap(new LinkedHashMap<>())); requestStatusMap = Collections.unmodifiableMap(map); + + expensiveTaskQueue = new ConcurrentLinkedQueue<>(); } public void shutdown() { + + shutdown = true; + + // Before shutting down the thread pool, we have to execute all the expensive tasks that + // are already in the queue + synchronized (this) { + if (!expensiveTaskQueue.isEmpty()) { + try { + wait(60000L); Review Comment: comment: notifyAll is in finishTask() -- 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. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org