You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "markrmiller (via GitHub)" <gi...@apache.org> on 2026/06/01 02:22:08 UTC
[PR] Unify SolrIndexSearcher paths with CollectorManager integration [solr]
markrmiller opened a new pull request, #4487:
URL: https://github.com/apache/solr/pull/4487
Replaces the dual sequential/parallel branches in getDocListNC and getDocListAndSetNC with a single private searchAndCollect helper that internally selects between single-slice and multi-slice execution. The top-level methods no longer fork on MultiThreadedSearcher.allowMT. Also makes canceling a query when doing parallel segment search actually do something.
## Performance — this commit vs parent
1.5M docs / 150 segments, 300 samples per cell per build, combined from two independent round-robin sessions (parent and this commit runs alternated within each session against a single shared `solr-home`). 30 warmup + 30 timed runs per cell per round. `sudo powermetrics --samplers cpu_power,thermal` ran in parallel at 1s cadence for both sessions; thermal verification below.
```
cell parent ms±se this commit ms±se delta% z
-----------------------------------------------------------------------------------------------------------------------------
match-all-top500 baseline mt_false 17.98±0.042 17.72±0.025 -1.4% -5.4
match-all-top500 baseline mt_true 17.85±0.039 17.73±0.026 -0.7% -2.7
match-all-top500 ste mt_false 10.19±0.027 10.09±0.022 -1.0% -2.8
match-all-top500 ste mt_true 10.13±0.021 10.10±0.023 -0.3% -0.8
match-all-top500 postfilter mt_false 25.54±0.045 25.78±0.083 +0.9% +2.5
match-all-top500 postfilter mt_true 25.41±0.045 25.62±0.033 +0.8% +3.8
match-all-top500 both mt_false 19.94±0.044 20.23±0.029 +1.5% +5.6
match-all-top500 both mt_true 20.12±0.080 20.23±0.035 +0.5% +1.2
big-bool-or-top1000 baseline mt_false 24.76±0.053 24.88±0.038 +0.5% +1.8
big-bool-or-top1000 baseline mt_true 26.31±0.046 26.31±0.038 -0.0% -0.1
big-bool-or-top1000 ste mt_false 5.66±0.036 5.58±0.020 -1.5% -2.1
big-bool-or-top1000 ste mt_true 5.61±0.027 5.49±0.020 -2.2% -3.6
big-bool-or-top1000 postfilter mt_false 30.49±0.040 30.55±0.040 +0.2% +1.0
big-bool-or-top1000 postfilter mt_true 30.47±0.041 30.58±0.036 +0.4% +2.0
big-bool-or-top1000 both mt_false 6.96±0.023 6.89±0.021 -1.0% -2.3
big-bool-or-top1000 both mt_true 6.89±0.037 6.98±0.022 +1.3% +2.1
sort-by-rank-top500 baseline mt_false 17.73±0.025 17.83±0.026 +0.6% +2.9
sort-by-rank-top500 baseline mt_true 17.76±0.025 17.78±0.027 +0.1% +0.6
sort-by-rank-top500 ste mt_false 17.40±0.021 17.37±0.024 -0.2% -0.9
sort-by-rank-top500 ste mt_true 17.39±0.022 17.37±0.024 -0.2% -0.9
sort-by-rank-top500 postfilter mt_false 25.86±0.065 25.67±0.034 -0.8% -2.7
sort-by-rank-top500 postfilter mt_true 25.74±0.030 25.79±0.080 +0.2% +0.6
sort-by-rank-top500 both mt_false 23.98±0.030 23.90±0.028 -0.3% -2.0
sort-by-rank-top500 both mt_true 23.93±0.027 23.96±0.030 +0.1% +0.7
big-bool-or-score-top500 baseline mt_false 54.93±0.070 55.05±0.110 +0.2% +0.9
big-bool-or-score-top500 baseline mt_true 53.37±0.063 53.54±0.088 +0.3% +1.5
big-bool-or-score-top500 ste mt_false 19.49±0.072 19.60±0.112 +0.5% +0.8
big-bool-or-score-top500 ste mt_true 19.52±0.161 19.50±0.073 -0.1% -0.1
big-bool-or-score-top500 postfilter mt_false 59.68±0.167 59.63±0.065 -0.1% -0.3
big-bool-or-score-top500 postfilter mt_true 59.42±0.056 59.69±0.102 +0.4% +2.3
big-bool-or-score-top500 both mt_false 20.41±0.174 20.66±0.252 +1.2% +0.8
big-bool-or-score-top500 both mt_true 20.15±0.039 20.20±0.044 +0.2% +0.8
```
Negative delta = this commit faster than parent.
`sudo powermetrics --samplers cpu_power,thermal -i 1000` ran in parallel for both sessions (~13 min each).
| build | sessions | runs | P-cluster avg MHz | CPU_W avg | thermal pressure |
|---|---:|---:|---:|---:|---|
| parent | 2 | 10 | P0 3041, P1 3060 | 8.27 | Nominal (every sample) |
| patched | 2 | 10 | P0 3051, P1 3055 | 8.25 | Nominal (every sample) |
No `Fair`/`Serious`/`Critical` events for any sample in any run across either session.
--
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] Unify SolrIndexSearcher paths with CollectorManager integration [solr]
Posted by "markrmiller (via GitHub)" <gi...@apache.org>.
markrmiller closed pull request #4487: Unify SolrIndexSearcher paths with CollectorManager integration
URL: https://github.com/apache/solr/pull/4487
--
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] Unify SolrIndexSearcher paths with CollectorManager integration [solr]
Posted by "HoustonPutman (via GitHub)" <gi...@apache.org>.
HoustonPutman commented on code in PR #4487:
URL: https://github.com/apache/solr/pull/4487#discussion_r3351600291
##########
solr/core/src/java/org/apache/solr/search/SolrMultiCollectorManager.java:
##########
Review Comment:
So it seems this copies a lot of code from Lucene. What needs to change in Lucene's `MultiCollectorManager` and `MultiCollector` such that we would be able to extend their classes without sacrificing our functionality?
--
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] Unify SolrIndexSearcher paths with CollectorManager integration [solr]
Posted by "dsmiley (via GitHub)" <gi...@apache.org>.
dsmiley commented on code in PR #4487:
URL: https://github.com/apache/solr/pull/4487#discussion_r3338900179
##########
solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java:
##########
Review Comment:
surprisingly a net increase in LOC ... but more documentation so that's good
##########
solr/core/src/java/org/apache/solr/search/SolrCollectorManagers.java:
##########
@@ -0,0 +1,602 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.search;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.LongAdder;
+import org.apache.lucene.index.LeafReaderContext;
+import org.apache.lucene.search.Collector;
+import org.apache.lucene.search.CollectorManager;
+import org.apache.lucene.search.ScoreMode;
+import org.apache.lucene.search.SimpleCollector;
+import org.apache.lucene.search.Sort;
+import org.apache.lucene.search.TopDocs;
+import org.apache.lucene.search.TopDocsCollector;
+import org.apache.lucene.search.TopFieldDocs;
+import org.apache.lucene.util.FixedBitSet;
+import org.apache.solr.core.CancellableQueryTracker;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Holders for the {@link CollectorManager} implementations used by Solr's search paths. Shared
+ * across the single-slice and multi-slice (parallel-segment) execution paths.
+ */
+final class SolrCollectorManagers {
+
+ private SolrCollectorManagers() {}
+
+ static class MaxScoreResult {
+ final float maxScore;
+
+ MaxScoreResult(float maxScore) {
+ this.maxScore = maxScore;
+ }
+ }
+
+ static class TopDocsResult {
+ final TopDocs topDocs;
+ final int totalHits;
+
+ TopDocsResult(TopDocs topDocs, int totalHits) {
+ this.topDocs = topDocs;
+ this.totalHits = totalHits;
+ }
+ }
+
+ static class SearchResult {
+ final ScoreMode scoreMode;
+ private final Object[] result;
+
+ SearchResult(ScoreMode scoreMode, Object[] result) {
+ this.scoreMode = scoreMode;
+ this.result = result;
+ }
+
+ public TopDocsResult getTopDocsResult() {
+ for (Object res : result) {
+ if (res instanceof TopDocsResult) {
+ return (TopDocsResult) res;
+ }
+ }
+ return null;
+ }
+
+ public float getMaxScore(int totalHits) {
+ if (totalHits > 0) {
+ for (Object res : result) {
+ if (res instanceof MaxScoreResult) {
+ return ((MaxScoreResult) res).maxScore;
+ }
+ }
+ return Float.NaN;
+ } else {
+ return 0.0f;
+ }
+ }
+
+ public DocSet getDocSet() {
+ for (Object res : result) {
+ if (res instanceof DocSet) {
+ return (DocSet) res;
+ }
+ if (res instanceof FixedBitSet fbs) {
+ return new BitDocSet(fbs);
+ }
+ }
+ return null;
+ }
+ }
+
+ static class FixedBitSetCollector extends SimpleCollector {
Review Comment:
could use a bit more docs/comments to explain what's going on
--
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