You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "dsmiley (via GitHub)" <gi...@apache.org> on 2025/02/02 05:04:22 UTC
[PR] SOLR-17648: multiThreaded=true: changed queue implementation [solr]
dsmiley opened a new pull request, #3155:
URL: https://github.com/apache/solr/pull/3155
from unlimited to 1000 max, after which the caller thread will execute.
Didn't need the RejectedExecutionException avoidance hack anymore; Lucene 9.12 has it.
https://issues.apache.org/jira/browse/SOLR-17648
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17648: multiThreaded=true: changed queue implementation [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley merged PR #3155:
URL: https://github.com/apache/solr/pull/3155
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17648: multiThreaded=true: changed queue implementation [solr]
Posted by "sigram (via GitHub)" <gi...@apache.org>.
sigram commented on code in PR #3155:
URL: https://github.com/apache/solr/pull/3155#discussion_r1939248091
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -228,25 +229,15 @@ public static ExecutorService initCollectorExecutor(NodeConfig cfg) {
return null;
}
+ // note that Lucene will catch a RejectedExecutionException to just run the task.
+ // Therefore, we shouldn't worry too much about the queue size.
return new MDCAwareThreadPoolExecutor(
indexSearcherExecutorThreads,
indexSearcherExecutorThreads,
0L,
TimeUnit.MILLISECONDS,
- new SynchronousQueue<>(true) { // fairness
- // a hack to force ThreadPoolExecutor to block if threads are busy
- // -- otherwise it will throw RejectedExecutionException; unacceptable
- @Override
- public boolean offer(Runnable runnable) { // is supposed to not block, but we do anyway
- try {
- put(runnable); // blocks
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- throw new RuntimeException("interrupted submitting to search multi-threaded pool", e);
- }
- return true;
- }
- },
+ new LinkedBlockingQueue<>(
+ EnvUtils.getPropertyAsInteger("solr.searcherCollector.queueSize", 1000)),
Review Comment:
Needs docs?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-17648: multiThreaded=true: changed queue implementation [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #3155:
URL: https://github.com/apache/solr/pull/3155#discussion_r1939755482
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
@@ -228,25 +229,15 @@ public static ExecutorService initCollectorExecutor(NodeConfig cfg) {
return null;
}
+ // note that Lucene will catch a RejectedExecutionException to just run the task.
+ // Therefore, we shouldn't worry too much about the queue size.
return new MDCAwareThreadPoolExecutor(
indexSearcherExecutorThreads,
indexSearcherExecutorThreads,
0L,
TimeUnit.MILLISECONDS,
- new SynchronousQueue<>(true) { // fairness
- // a hack to force ThreadPoolExecutor to block if threads are busy
- // -- otherwise it will throw RejectedExecutionException; unacceptable
- @Override
- public boolean offer(Runnable runnable) { // is supposed to not block, but we do anyway
- try {
- put(runnable); // blocks
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- throw new RuntimeException("interrupted submitting to search multi-threaded pool", e);
- }
- return true;
- }
- },
+ new LinkedBlockingQueue<>(
+ EnvUtils.getPropertyAsInteger("solr.searcherCollector.queueSize", 1000)),
Review Comment:
IMO we don't need to document _obscure_ settings (I think this is). If you think it should be documented, let me know where it should be. If this queue size is exceeded, the outcome is still a working system but the performance will be somewhat less than what it otherwise would be. FWIW 1000 feels about right to me. I reviewed the code in Lucene 9.12 that cleverly uses the parent thread.
I plan to change this property to be "solr.search.multiThreaded.queueSize" so as to have a category after solr. -- see https://cwiki.apache.org/confluence/display/SOLR/System+property+naming+structure
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org