dsmiley commented on code in PR #2248: URL: https://github.com/apache/solr/pull/2248#discussion_r1485475990
########## solr/core/src/java/org/apache/solr/search/SolrMultiCollectorManager.java: ########## @@ -0,0 +1,150 @@ +/* + * 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.util.ArrayList; +import java.util.Collection; +import java.util.List; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Collector; +import org.apache.lucene.search.CollectorManager; +import org.apache.lucene.search.FilterScorable; +import org.apache.lucene.search.LeafCollector; +import org.apache.lucene.search.MultiCollector; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.ScoreMode; + +/** + * A {@link CollectorManager} implements which wrap a set of {@link CollectorManager} as {@link + * MultiCollector} acts for {@link Collector}. + */ +public class SolrMultiCollectorManager + implements CollectorManager<SolrMultiCollectorManager.Collectors, Object[]> { + + private final CollectorManager<Collector, ?>[] collectorManagers; + + @SafeVarargs + @SuppressWarnings({"varargs", "unchecked"}) + public SolrMultiCollectorManager( + final CollectorManager<? extends Collector, ?>... collectorManagers) { + if (collectorManagers.length < 1) { + throw new IllegalArgumentException("There must be at least one collector"); + } + this.collectorManagers = (CollectorManager[]) collectorManagers; + } + + @Override + public Collectors newCollector() throws IOException { + return new Collectors(); + } + + @Override + public Object[] reduce(Collection<Collectors> reducableCollectors) throws IOException { + final int size = reducableCollectors.size(); + final Object[] results = new Object[collectorManagers.length]; + for (int i = 0; i < collectorManagers.length; i++) { + final List<Collector> reducableCollector = new ArrayList<>(size); + for (Collectors collectors : reducableCollectors) + reducableCollector.add(collectors.collectors[i]); + results[i] = collectorManagers[i].reduce(reducableCollector); + } + return results; + } + + /** Wraps multiple collectors for processing */ + public class Collectors implements Collector { Review Comment: not sure yet if these inner classes need to be public. ########## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ########## @@ -1867,35 +1873,79 @@ public ScoreMode scoreMode() { qr.setNextCursorMark(cmd.getCursorMark()); hitsRelation = Relation.EQUAL_TO; } else { - final TopDocsCollector<?> topCollector = buildTopDocsCollector(len, cmd); - MaxScoreCollector maxScoreCollector = null; - Collector collector = topCollector; - if ((cmd.getFlags() & GET_SCORES) != 0) { - maxScoreCollector = new MaxScoreCollector(); - collector = MultiCollector.wrap(topCollector, maxScoreCollector); + if (log.isInfoEnabled()) { + log.info("calling from 2, query: {}", query.getClass()); } - ScoreMode scoreModeUsed = - buildAndRunCollectorChain(qr, query, collector, cmd, pf.postFilter).scoreMode(); + MTCollectorQueryCheck allowMT = new MTCollectorQueryCheck(); Review Comment: Bunch of code being added in two places looking similar. Have you thought of attempting to factor out a common approach? ########## solr/core/src/java/org/apache/solr/search/ThreadSafeBitSetCollector.java: ########## @@ -0,0 +1,90 @@ +/* + * 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 org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.search.SimpleCollector; +import org.apache.lucene.util.FixedBitSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** */ Review Comment: ```suggestion /** A Lucene Collector collecting docs in a thread-safe manner. */ ``` ########## solr/core/src/java/org/apache/solr/search/ThreadSafeBitSetCollector.java: ########## @@ -0,0 +1,90 @@ +/* + * 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 org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.search.SimpleCollector; +import org.apache.lucene.util.FixedBitSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** */ +public class ThreadSafeBitSetCollector extends SimpleCollector { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + final ThreadSafeBitSet bits; + final int maxDoc; + + int base; + + public ThreadSafeBitSetCollector(ThreadSafeBitSet bits, int maxDoc) { + this.bits = bits; + this.maxDoc = maxDoc; + } + + @Override + public void collect(int doc) throws IOException { + + doc += base; + if (log.isErrorEnabled()) { + log.error("collect doc: {}, base: {}", doc, base, this); + } + bits.set(doc); + } + + /** The number of documents that have been collected */ + public int size() { + return maxDoc; + } + + public DocSet getDocSet() { + log.error("Max Set Bit {}", bits.maxSetBit()); Review Comment: error? ########## solr/core/src/java/org/apache/solr/search/ThreadSafeBitSetCollector.java: ########## @@ -0,0 +1,90 @@ +/* + * 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 org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.search.SimpleCollector; +import org.apache.lucene.util.FixedBitSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** */ +public class ThreadSafeBitSetCollector extends SimpleCollector { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + final ThreadSafeBitSet bits; + final int maxDoc; + + int base; + + public ThreadSafeBitSetCollector(ThreadSafeBitSet bits, int maxDoc) { + this.bits = bits; + this.maxDoc = maxDoc; + } + + @Override + public void collect(int doc) throws IOException { + + doc += base; + if (log.isErrorEnabled()) { + log.error("collect doc: {}, base: {}", doc, base, this); Review Comment: error? ########## solr/core/src/java/org/apache/solr/search/QueryResult.java: ########## @@ -46,6 +46,7 @@ public void setDocSet(DocSet set) { docListAndSet = new DocListAndSet(); } docListAndSet.docSet = set; + // log.error("set docset {}", docListAndSet.docSet.getBits().length()); Review Comment: should remove ########## solr/core/src/java/org/apache/solr/search/ThreadSafeBitSetCollector.java: ########## @@ -0,0 +1,90 @@ +/* + * 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 org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.search.SimpleCollector; +import org.apache.lucene.util.FixedBitSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** */ +public class ThreadSafeBitSetCollector extends SimpleCollector { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + final ThreadSafeBitSet bits; + final int maxDoc; + + int base; + + public ThreadSafeBitSetCollector(ThreadSafeBitSet bits, int maxDoc) { + this.bits = bits; + this.maxDoc = maxDoc; + } + + @Override + public void collect(int doc) throws IOException { + + doc += base; + if (log.isErrorEnabled()) { + log.error("collect doc: {}, base: {}", doc, base, this); + } + bits.set(doc); + } + + /** The number of documents that have been collected */ + public int size() { + return maxDoc; + } + + public DocSet getDocSet() { + log.error("Max Set Bit {}", bits.maxSetBit()); + + FixedBitSet fixedBitSet = new FixedBitSet(maxDoc + 1); + int cnt = 0; + int i = -1; + while (true) { + i = bits.nextSetBit(i + 1); + if (i == -1) { + break; + } + cnt++; + fixedBitSet.set(i); + } + + return new BitDocSet(fixedBitSet, cnt); + } + + @Override + public void setScorer(Scorable scorer) throws IOException {} + + @Override + public ScoreMode scoreMode() { + return ScoreMode.COMPLETE_NO_SCORES; + } + + @Override + protected void doSetNextReader(LeafReaderContext context) throws IOException { + this.base = context.docBase; + if (log.isErrorEnabled()) { + log.error("next reader base={}", base); Review Comment: error? BTW I recommend "nocommit" for this stuff ########## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ########## @@ -1910,6 +1960,203 @@ public ScoreMode scoreMode() { qr.setDocList(new DocSlice(0, sliceLen, ids, scores, totalHits, maxScore, hitsRelation)); } + SearchResult searchCollectorManagers( + int len, + QueryCommand cmd, + Query query, + boolean needTopDocs, + boolean needMaxScore, + boolean needDocSet) + throws IOException { + Collection<CollectorManager<Collector, Object>> collectors = new ArrayList<>(); + ScoreMode scoreMode = null; + + Collector[] firstCollectors = new Collector[3]; + + if (needTopDocs) { + + collectors.add( + new CollectorManager<>() { + @Override + public Collector newCollector() throws IOException { + @SuppressWarnings("rawtypes") + TopDocsCollector collector = buildTopDocsCollector(len, cmd); + if (firstCollectors[0] == null) { + firstCollectors[0] = collector; + } + return collector; + } + + @Override + @SuppressWarnings("rawtypes") + public Object reduce(Collection collectors) throws IOException { + + TopDocs[] topDocs = new TopDocs[collectors.size()]; + + int totalHits = -1; + int i = 0; + + Collector collector; + for (Object o : collectors) { + collector = (Collector) o; + if (collector instanceof TopDocsCollector) { + TopDocs td = ((TopDocsCollector) collector).topDocs(0, len); + assert td != null : Arrays.asList(topDocs); + topDocs[i++] = td; + } + } + + TopDocs mergedTopDocs = null; + + if (topDocs.length > 0 && topDocs[0] != null) { + if (topDocs[0] instanceof TopFieldDocs) { + TopFieldDocs[] topFieldDocs = + Arrays.copyOf(topDocs, topDocs.length, TopFieldDocs[].class); + mergedTopDocs = TopFieldDocs.merge(weightSort(cmd.getSort()), len, topFieldDocs); + } else { + mergedTopDocs = TopDocs.merge(0, len, topDocs); + } + totalHits = (int) mergedTopDocs.totalHits.value; + } + return new TopDocsResult(mergedTopDocs, totalHits); + } + }); + } + if (needMaxScore) { + collectors.add( + new CollectorManager<>() { + @Override + public Collector newCollector() throws IOException { + MaxScoreCollector collector = new MaxScoreCollector(); + if (firstCollectors[1] == null) { + firstCollectors[1] = collector; + } + return collector; + } + + @Override + @SuppressWarnings("rawtypes") + public Object reduce(Collection collectors) throws IOException { + + MaxScoreCollector collector; + float maxScore = 0.0f; + for (Iterator var4 = collectors.iterator(); + var4.hasNext(); + maxScore = Math.max(maxScore, collector.getMaxScore())) { + collector = (MaxScoreCollector) var4.next(); + } + + return new MaxScoreResult(maxScore); + } + }); + } + if (needDocSet) { + int maxDoc = rawReader.maxDoc(); + log.error("raw read max={}", rawReader.maxDoc()); + + LeafSlice[] leaves = getSlices(); + int[] docBase = new int[1]; + + // DocSetCollector collector = new DocSetCollector(maxDoc); + + ThreadSafeBitSet bits = new ThreadSafeBitSet(14, 2); + + collectors.add( + new CollectorManager<>() { + @Override + public Collector newCollector() throws IOException { + int numDocs = 0; + + if (leaves != null) { + LeafSlice leaf = leaves[docBase[0]++]; + + for (LeafReaderContext reader : leaf.leaves) { + numDocs += reader.reader().maxDoc(); + } + } else { + numDocs = maxDoc(); + } + log.error("new docset collector for {} max={}", numDocs, maxDoc()); + + return new ThreadSafeBitSetCollector(bits, maxDoc); + } + + @Override + @SuppressWarnings({"rawtypes"}) + public Object reduce(Collection collectors) throws IOException { + + return new DocSetResult( + ((ThreadSafeBitSetCollector) collectors.iterator().next()).getDocSet()); + } + }); + } + for (Collector collector : firstCollectors) { + if (collector != null) { + if (scoreMode == null) { + scoreMode = collector.scoreMode(); + } else if (scoreMode != collector.scoreMode()) { + scoreMode = ScoreMode.COMPLETE; + } + } + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + CollectorManager<Collector, Object>[] colls = collectors.toArray(new CollectorManager[0]); + SolrMultiCollectorManager manager = new SolrMultiCollectorManager(colls); + Object[] ret; + try { + ret = super.search(query, manager); + } catch (Exception ex) { + if (ex instanceof RuntimeException + && ex.getCause() != null + && ex.getCause() instanceof ExecutionException + && ex.getCause().getCause() != null + && ex.getCause().getCause() instanceof RuntimeException) { + throw (RuntimeException) ex.getCause().getCause(); + } else { + throw ex; + } + } + + return new SearchResult(scoreMode, ret); + } + + static class TopDocsResult { + final TopDocs topDocs; + final int totalHits; + + public TopDocsResult(TopDocs topDocs, int totalHits) { + this.topDocs = topDocs; + this.totalHits = totalHits; + } + } + + static class MaxScoreResult { + final float maxScore; + + public MaxScoreResult(float maxScore) { + this.maxScore = maxScore; + } + } + + static class DocSetResult { + final DocSet docSet; + + public DocSetResult(DocSet docSet) { + this.docSet = docSet; + } + } + + static class SearchResult { + final Object[] result; Review Comment: type Object? ########## solr/core/src/java/org/apache/solr/search/ThreadSafeBitSetCollector.java: ########## @@ -0,0 +1,90 @@ +/* + * 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 org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.ScoreMode; +import org.apache.lucene.search.SimpleCollector; +import org.apache.lucene.util.FixedBitSet; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** */ +public class ThreadSafeBitSetCollector extends SimpleCollector { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + final ThreadSafeBitSet bits; + final int maxDoc; + + int base; + + public ThreadSafeBitSetCollector(ThreadSafeBitSet bits, int maxDoc) { + this.bits = bits; + this.maxDoc = maxDoc; + } + + @Override + public void collect(int doc) throws IOException { + + doc += base; + if (log.isErrorEnabled()) { + log.error("collect doc: {}, base: {}", doc, base, this); + } + bits.set(doc); + } + + /** The number of documents that have been collected */ + public int size() { + return maxDoc; + } + + public DocSet getDocSet() { + log.error("Max Set Bit {}", bits.maxSetBit()); + + FixedBitSet fixedBitSet = new FixedBitSet(maxDoc + 1); + int cnt = 0; + int i = -1; + while (true) { + i = bits.nextSetBit(i + 1); + if (i == -1) { + break; + } + cnt++; + fixedBitSet.set(i); + } + + return new BitDocSet(fixedBitSet, cnt); Review Comment: Okay for now but obviously could be more optimized -- deserves a TODO. We should have ThreadSafeBitSet be able to return a FixedBitSet directly by copying the longs instead of working a set bit at a time. ########## solr/core/src/test/org/apache/solr/search/TestFiltering.java: ########## @@ -92,21 +93,34 @@ public void testLiveDocsSharing() throws Exception { QueryResult res = new QueryResult(); searcher.search(res, cmd); set = res.getDocSet(); - assertSame(set, live); + assertEffectivelySame(set.getFixedBitSet(), live.getFixedBitSet()); cmd.setQuery(QParser.getParser(qstr + " OR id:0", null, req).getQuery()); cmd.setFilterList(QParser.getParser(qstr + " OR id:1", null, req).getQuery()); res = new QueryResult(); searcher.search(res, cmd); set = res.getDocSet(); - assertSame(set, live); + assertEffectivelySame(set.getFixedBitSet(), live.getFixedBitSet()); } } finally { req.close(); } } + /** If the a XOR b == 0, then both a & b are effectively the same bitset */ + private void assertEffectivelySame(FixedBitSet a, FixedBitSet b) { Review Comment: Can you explain why assertSame is no longer a working in this test? I remember working on this with Yonik. The very point of this test is to test that we use the actual instance of live docs (notice the test method's name) ########## solr/solr-ref-guide/modules/indexing-guide/examples/stemdict.txt: ########## Review Comment: Looks like you removed a symlink? ########## solr/core/src/java/org/apache/solr/search/SolrMultiCollectorManager.java: ########## @@ -0,0 +1,150 @@ +/* + * 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.util.ArrayList; +import java.util.Collection; +import java.util.List; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Collector; +import org.apache.lucene.search.CollectorManager; +import org.apache.lucene.search.FilterScorable; +import org.apache.lucene.search.LeafCollector; +import org.apache.lucene.search.MultiCollector; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.ScoreMode; + +/** + * A {@link CollectorManager} implements which wrap a set of {@link CollectorManager} as {@link + * MultiCollector} acts for {@link Collector}. + */ +public class SolrMultiCollectorManager + implements CollectorManager<SolrMultiCollectorManager.Collectors, Object[]> { Review Comment: Hmm; Object? Do we not know the type? ########## solr/core/src/java/org/apache/solr/search/SolrMultiCollectorManager.java: ########## @@ -0,0 +1,150 @@ +/* + * 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.util.ArrayList; +import java.util.Collection; +import java.util.List; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.search.Collector; +import org.apache.lucene.search.CollectorManager; +import org.apache.lucene.search.FilterScorable; +import org.apache.lucene.search.LeafCollector; +import org.apache.lucene.search.MultiCollector; +import org.apache.lucene.search.Scorable; +import org.apache.lucene.search.ScoreMode; + +/** + * A {@link CollectorManager} implements which wrap a set of {@link CollectorManager} as {@link Review Comment: ```suggestion * A {@link CollectorManager} implementation which wrap a set of {@link CollectorManager} as {@link ``` ########## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ########## @@ -1979,36 +2226,84 @@ public ScoreMode scoreMode() { // no docs on this page, so cursor doesn't change qr.setNextCursorMark(cmd.getCursorMark()); } else { - final TopDocsCollector<? extends ScoreDoc> topCollector = buildTopDocsCollector(len, cmd); - DocSetCollector setCollector = new DocSetCollector(maxDoc); - MaxScoreCollector maxScoreCollector = null; - List<Collector> collectors = new ArrayList<>(Arrays.asList(topCollector, setCollector)); - - if ((cmd.getFlags() & GET_SCORES) != 0) { - maxScoreCollector = new MaxScoreCollector(); - collectors.add(maxScoreCollector); - } + MTCollectorQueryCheck allowMT = new MTCollectorQueryCheck(); + query.visit(allowMT); + TopDocs topDocs; + if (pf.postFilter != null + || cmd.getSegmentTerminateEarly() + || cmd.getTimeAllowed() > 0 + || !allowMT.allowed()) { + @SuppressWarnings({"rawtypes"}) + final TopDocsCollector<? extends ScoreDoc> topCollector = buildTopDocsCollector(len, cmd); + DocSetCollector setCollector = new DocSetCollector(maxDoc); + MaxScoreCollector maxScoreCollector = null; + List<Collector> collectors = new ArrayList<>(Arrays.asList(topCollector, setCollector)); + + if ((cmd.getFlags() & GET_SCORES) != 0) { + maxScoreCollector = new MaxScoreCollector(); + collectors.add(maxScoreCollector); + } - Collector collector = MultiCollector.wrap(collectors); + Collector collector = MultiCollector.wrap(collectors); - buildAndRunCollectorChain(qr, query, collector, cmd, pf.postFilter); + buildAndRunCollectorChain(qr, query, collector, cmd, pf.postFilter); - set = DocSetUtil.getDocSet(setCollector, this); + set = DocSetUtil.getDocSet(setCollector, this); + + totalHits = topCollector.getTotalHits(); + assert (totalHits == set.size()) || qr.isPartialResults(); - totalHits = topCollector.getTotalHits(); - assert (totalHits == set.size()) || qr.isPartialResults(); + topDocs = topCollector.topDocs(0, len); + if (cmd.getSort() != null + && !(cmd.getQuery() instanceof RankQuery) + && (cmd.getFlags() & GET_SCORES) != 0) { + TopFieldCollector.populateScores(topDocs.scoreDocs, this, query); + } + populateNextCursorMarkFromTopDocs(qr, cmd, topDocs); + maxScore = + totalHits > 0 + ? (maxScoreCollector == null ? Float.NaN : maxScoreCollector.getMaxScore()) + : 0.0f; + } else { + log.debug("using CollectorManager"); + + boolean needMaxScore = (cmd.getFlags() & GET_SCORES) != 0; + SearchResult searchResult = + searchCollectorManagers(len, cmd, query, true, needMaxScore, true); + Object[] res = searchResult.result; + TopDocsResult result = (TopDocsResult) res[0]; + totalHits = result.totalHits; + topDocs = result.topDocs; + if (needMaxScore) { + if (res.length > 1) { + MaxScoreResult result2 = (MaxScoreResult) res[1]; + maxScore = totalHits > 0 ? result2.maxScore : 0.0f; + } + if (res.length > 2) { + DocSetResult result3 = (DocSetResult) res[2]; + set = result3.docSet; + } + } else { + if (res.length > 1) { + DocSetResult result2 = (DocSetResult) res[1]; + set = result2.docSet; + } + } + + populateNextCursorMarkFromTopDocs(qr, cmd, topDocs); + // if (cmd.getSort() != null && !(cmd.getQuery() instanceof RankQuery) && Review Comment: Forgot some logic or...? ########## solr/core/src/test/org/apache/solr/TestDistributedSearch.java: ########## @@ -1786,7 +1786,7 @@ private void testMinExactCount() throws Exception { CommonParams.ROWS, "200", CommonParams.SORT, - "score desc, id asc"); + "id asc"); Review Comment: What's this about? ########## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ########## @@ -1867,35 +1873,79 @@ public ScoreMode scoreMode() { qr.setNextCursorMark(cmd.getCursorMark()); hitsRelation = Relation.EQUAL_TO; } else { - final TopDocsCollector<?> topCollector = buildTopDocsCollector(len, cmd); - MaxScoreCollector maxScoreCollector = null; - Collector collector = topCollector; - if ((cmd.getFlags() & GET_SCORES) != 0) { - maxScoreCollector = new MaxScoreCollector(); - collector = MultiCollector.wrap(topCollector, maxScoreCollector); + if (log.isInfoEnabled()) { + log.info("calling from 2, query: {}", query.getClass()); } - ScoreMode scoreModeUsed = - buildAndRunCollectorChain(qr, query, collector, cmd, pf.postFilter).scoreMode(); + MTCollectorQueryCheck allowMT = new MTCollectorQueryCheck(); + query.visit(allowMT); + TopDocs topDocs; + if (pf.postFilter != null + || cmd.getSegmentTerminateEarly() + || cmd.getTimeAllowed() > 0 + || !allowMT.allowed()) { + if (log.isInfoEnabled()) { + log.info("skipping collector manager"); + } + final TopDocsCollector<?> topCollector = buildTopDocsCollector(len, cmd); + MaxScoreCollector maxScoreCollector = null; + Collector collector = topCollector; + if (needScores) { + maxScoreCollector = new MaxScoreCollector(); + collector = MultiCollector.wrap(topCollector, maxScoreCollector); + } + ScoreMode scoreModeUsed = + buildAndRunCollectorChain(qr, query, collector, cmd, pf.postFilter).scoreMode(); + + totalHits = topCollector.getTotalHits(); + topDocs = topCollector.topDocs(0, len); + if (scoreModeUsed == ScoreMode.COMPLETE || scoreModeUsed == ScoreMode.COMPLETE_NO_SCORES) { + hitsRelation = TotalHits.Relation.EQUAL_TO; + } else { + hitsRelation = topDocs.totalHits.relation; + } + if (cmd.getSort() != null && cmd.getQuery() instanceof RankQuery == false && needScores) { + TopFieldCollector.populateScores(topDocs.scoreDocs, this, query); + } + populateNextCursorMarkFromTopDocs(qr, cmd, topDocs); + + maxScore = + totalHits > 0 + ? (maxScoreCollector == null ? Float.NaN : maxScoreCollector.getMaxScore()) + : 0.0f; + nDocsReturned = topDocs.scoreDocs.length; - totalHits = topCollector.getTotalHits(); - TopDocs topDocs = topCollector.topDocs(0, len); - if (scoreModeUsed == ScoreMode.COMPLETE || scoreModeUsed == ScoreMode.COMPLETE_NO_SCORES) { - hitsRelation = TotalHits.Relation.EQUAL_TO; } else { - hitsRelation = topDocs.totalHits.relation; - } - if (cmd.getSort() != null - && cmd.getQuery() instanceof RankQuery == false - && (cmd.getFlags() & GET_SCORES) != 0) { - TopFieldCollector.populateScores(topDocs.scoreDocs, this, query); + if (log.isInfoEnabled()) { + log.info("using CollectorManager"); Review Comment: more debugging code ########## solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java: ########## @@ -1910,6 +1960,203 @@ public ScoreMode scoreMode() { qr.setDocList(new DocSlice(0, sliceLen, ids, scores, totalHits, maxScore, hitsRelation)); } + SearchResult searchCollectorManagers( + int len, + QueryCommand cmd, + Query query, + boolean needTopDocs, + boolean needMaxScore, + boolean needDocSet) + throws IOException { + Collection<CollectorManager<Collector, Object>> collectors = new ArrayList<>(); + ScoreMode scoreMode = null; + + Collector[] firstCollectors = new Collector[3]; + + if (needTopDocs) { + + collectors.add( + new CollectorManager<>() { + @Override + public Collector newCollector() throws IOException { + @SuppressWarnings("rawtypes") + TopDocsCollector collector = buildTopDocsCollector(len, cmd); + if (firstCollectors[0] == null) { + firstCollectors[0] = collector; + } + return collector; + } + + @Override + @SuppressWarnings("rawtypes") + public Object reduce(Collection collectors) throws IOException { + + TopDocs[] topDocs = new TopDocs[collectors.size()]; + + int totalHits = -1; + int i = 0; + + Collector collector; + for (Object o : collectors) { + collector = (Collector) o; + if (collector instanceof TopDocsCollector) { + TopDocs td = ((TopDocsCollector) collector).topDocs(0, len); + assert td != null : Arrays.asList(topDocs); + topDocs[i++] = td; + } + } + + TopDocs mergedTopDocs = null; + + if (topDocs.length > 0 && topDocs[0] != null) { + if (topDocs[0] instanceof TopFieldDocs) { + TopFieldDocs[] topFieldDocs = + Arrays.copyOf(topDocs, topDocs.length, TopFieldDocs[].class); + mergedTopDocs = TopFieldDocs.merge(weightSort(cmd.getSort()), len, topFieldDocs); + } else { + mergedTopDocs = TopDocs.merge(0, len, topDocs); + } + totalHits = (int) mergedTopDocs.totalHits.value; + } + return new TopDocsResult(mergedTopDocs, totalHits); + } + }); + } + if (needMaxScore) { + collectors.add( + new CollectorManager<>() { + @Override + public Collector newCollector() throws IOException { + MaxScoreCollector collector = new MaxScoreCollector(); + if (firstCollectors[1] == null) { + firstCollectors[1] = collector; + } + return collector; + } + + @Override + @SuppressWarnings("rawtypes") + public Object reduce(Collection collectors) throws IOException { + + MaxScoreCollector collector; + float maxScore = 0.0f; + for (Iterator var4 = collectors.iterator(); + var4.hasNext(); + maxScore = Math.max(maxScore, collector.getMaxScore())) { + collector = (MaxScoreCollector) var4.next(); + } + + return new MaxScoreResult(maxScore); + } + }); + } + if (needDocSet) { + int maxDoc = rawReader.maxDoc(); + log.error("raw read max={}", rawReader.maxDoc()); + + LeafSlice[] leaves = getSlices(); + int[] docBase = new int[1]; + + // DocSetCollector collector = new DocSetCollector(maxDoc); + + ThreadSafeBitSet bits = new ThreadSafeBitSet(14, 2); + + collectors.add( + new CollectorManager<>() { + @Override + public Collector newCollector() throws IOException { + int numDocs = 0; + + if (leaves != null) { + LeafSlice leaf = leaves[docBase[0]++]; + + for (LeafReaderContext reader : leaf.leaves) { + numDocs += reader.reader().maxDoc(); + } + } else { + numDocs = maxDoc(); + } + log.error("new docset collector for {} max={}", numDocs, maxDoc()); + + return new ThreadSafeBitSetCollector(bits, maxDoc); + } + + @Override + @SuppressWarnings({"rawtypes"}) + public Object reduce(Collection collectors) throws IOException { + + return new DocSetResult( + ((ThreadSafeBitSetCollector) collectors.iterator().next()).getDocSet()); + } + }); + } + for (Collector collector : firstCollectors) { + if (collector != null) { + if (scoreMode == null) { + scoreMode = collector.scoreMode(); + } else if (scoreMode != collector.scoreMode()) { + scoreMode = ScoreMode.COMPLETE; + } + } + } Review Comment: I've seen this code at least twice in this patch, if I'm not mistaken. Could be a utility method. -- 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
