You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "cpoerschke (via GitHub)" <gi...@apache.org> on 2024/06/07 16:40:48 UTC
[PR] SOLR-13350: multi-threaded search: replace cached with fixed threadpool [solr]
cpoerschke opened a new pull request, #2508:
URL: https://github.com/apache/solr/pull/2508
https://issues.apache.org/jira/browse/SOLR-13350
--
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-13350: multi-threaded search: replace cached with fixed threadpool [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #2508:
URL: https://github.com/apache/solr/pull/2508#discussion_r1688330420
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -445,9 +445,9 @@ public CoreContainer(NodeConfig config, CoresLocator locator, boolean asyncSolrC
final int indexSearcherExecutorThreads = cfg.getIndexSearcherExecutorThreads();
if (0 < indexSearcherExecutorThreads) {
this.collectorExecutor =
- ExecutorUtil.newMDCAwareCachedThreadPool(
- indexSearcherExecutorThreads, // thread count
- indexSearcherExecutorThreads * 1000, // queue size
+ ExecutorUtil.newMDCAwareFixedThreadPool(
+ cfg.getIndexSearcherExecutorThreads(), // thread count
+ cfg.getIndexSearcherExecutorThreads() * 1000, // queue size
Review Comment:
oops, hasty merge conflict resolution, i missed this:
```suggestion
indexSearcherExecutorThreads, // thread count
indexSearcherExecutorThreads * 1000, // queue size
```
--
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-13350: multi-threaded search: replace cached with fixed threadpool [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on PR #2508:
URL: https://github.com/apache/solr/pull/2508#issuecomment-2176117831
Can you recommend a way to benchmark this myself, like if I wanted to tweak it to see how my ideas work out?
--
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-13350: multi-threaded search: replace cached with fixed threadpool [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on code in PR #2508:
URL: https://github.com/apache/solr/pull/2508#discussion_r1631463199
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -444,7 +444,7 @@ public CoreContainer(NodeConfig config, CoresLocator locator, boolean asyncSolrC
this.allowListUrlChecker = AllowListUrlChecker.create(config);
this.collectorExecutor =
- ExecutorUtil.newMDCAwareCachedThreadPool(
+ ExecutorUtil.newMDCAwareFixedThreadPool(
cfg.getIndexSearcherExecutorThreads(), // thread count
cfg.getIndexSearcherExecutorThreads() * 1000, // queue size
new SolrNamedThreadFactory("searcherCollector"));
Review Comment:
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/ThreadPoolExecutor.html documentation says _"If corePoolSize or more threads are running, the Executor always prefers queuing a request rather than adding a new thread."_ and _"If a request cannot be queued, a new thread is created unless this would exceed maximumPoolSize, in which case, the task will be rejected."_
I think this means that if the queue size is generous (relative to the use of the thread pool) then queuing will always be possible and search will remain single-threaded.
--
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-13350: multi-threaded search: replace cached with fixed threadpool [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke merged PR #2508:
URL: https://github.com/apache/solr/pull/2508
--
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-13350: multi-threaded search: replace cached with fixed threadpool [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on PR #2508:
URL: https://github.com/apache/solr/pull/2508#issuecomment-2176622969
> Can you recommend a way to benchmark this myself, like if I wanted to tweak it to see how my ideas work out?
I've shared the "just pass an executor" patch onto a Solr 9.5 cloud that I used, in case that's useful.
The benchmarking experiments I ran used Python's `threading` module and (I think under the hood) `pysolr` also, measuring `QTime`/elapsed time/CPU utilisation, for example. My experiments were for dense vector search, I haven't yet looked into if/how other searches might use the passed in executor at the Lucene level. But it would be worth mentioning at this point that the collection was quite multi-sharded i.e. that being independent of the type of search being run.
--
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-13350: multi-threaded search: replace cached with fixed threadpool [solr]
Posted by "cpoerschke (via GitHub)" <gi...@apache.org>.
cpoerschke commented on PR #2508:
URL: https://github.com/apache/solr/pull/2508#issuecomment-2237112708
> Along with this maybe we want to change
>
> ```
> public static final int DEFAULT_INDEX_SEARCHER_EXECUTOR_THREADS = 4;
> ```
>
> to
>
> ```
> public static final int DEFAULT_INDEX_SEARCHER_EXECUTOR_THREADS = Runtime.getRuntime().availableProcessors();
> ```
Opened #2569 to separate that less controversial (?) change out from the less obvious (?) change here. Also there's some associated doc and config changes needed to go with making the default runtime aware.
--
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