javanna commented on code in PR #13735:
URL: https://github.com/apache/lucene/pull/13735#discussion_r1750433222
##########
lucene/core/src/java/org/apache/lucene/search/CollectorManager.java:
##########
@@ -46,4 +48,28 @@ public interface CollectorManager<C extends Collector, T> {
* called after collection is finished on all provided collectors.
*/
T reduce(Collection<C> collectors) throws IOException;
+
+ /**
+ * Wrap a provided {@link Collector} with a thin {@code CollectorManager}
wrapper for use with
+ * {@link IndexSearcher#search(Query, CollectorManager)}. The wrapping
{@code CollectorManager}
+ * provides no {@link CollectorManager#reduce(Collection)} implementation,
so the wrapped {@code
+ * Collector} needs to do all relevant work while collecting.
+ *
+ * <p>Note: This is only safe to use when {@code IndexSearcher} is created
with no executor (see:
+ * {@link IndexSearcher#IndexSearcher(IndexReader, Executor)}), or the
provided collector is
+ * threadsafe.
+ */
+ static <C extends Collector> CollectorManager<C, ?> wrap(C in) {
+ return new CollectorManager<C, Void>() {
+ @Override
+ public C newCollector() {
+ return in;
+ }
+
+ @Override
+ public Void reduce(Collection<C> collectors) {
+ return null;
Review Comment:
assert that this is called providing a collection of size 1 that contains
exactly the wrapped collector? Actually I am wondering if these should be
IllegalStateExceptions instead of assert
##########
lucene/core/src/java/org/apache/lucene/search/CollectorManager.java:
##########
@@ -46,4 +48,28 @@ public interface CollectorManager<C extends Collector, T> {
* called after collection is finished on all provided collectors.
*/
T reduce(Collection<C> collectors) throws IOException;
+
+ /**
+ * Wrap a provided {@link Collector} with a thin {@code CollectorManager}
wrapper for use with
+ * {@link IndexSearcher#search(Query, CollectorManager)}. The wrapping
{@code CollectorManager}
+ * provides no {@link CollectorManager#reduce(Collection)} implementation,
so the wrapped {@code
+ * Collector} needs to do all relevant work while collecting.
+ *
+ * <p>Note: This is only safe to use when {@code IndexSearcher} is created
with no executor (see:
+ * {@link IndexSearcher#IndexSearcher(IndexReader, Executor)}), or the
provided collector is
+ * threadsafe.
+ */
+ static <C extends Collector> CollectorManager<C, ?> wrap(C in) {
Review Comment:
This is not fantastic but I suspect we may lean on this solution for other
leftover usages of Collector vs CollectorManager. Shall we clarify in the name
that this is for sequential execution / single threaded?
##########
lucene/monitor/src/java/org/apache/lucene/monitor/CollectingMatcher.java:
##########
@@ -37,7 +38,7 @@ abstract class CollectingMatcher<T extends QueryMatch>
extends CandidateMatcher<
@Override
public void matchQuery(final String queryId, Query matchQuery, Map<String,
String> metadata)
throws IOException {
- searcher.search(matchQuery, new MatchCollector(queryId, scoreMode));
+ searcher.search(matchQuery, CollectorManager.wrap(new
MatchCollector(queryId, scoreMode)));
Review Comment:
I left a comment about whether this is entirely safe, I was under the
impression that one can provide an external searcher but I am not familiar with
monitor code ;)
##########
lucene/core/src/java/org/apache/lucene/search/CollectorManager.java:
##########
@@ -46,4 +48,28 @@ public interface CollectorManager<C extends Collector, T> {
* called after collection is finished on all provided collectors.
*/
T reduce(Collection<C> collectors) throws IOException;
+
+ /**
+ * Wrap a provided {@link Collector} with a thin {@code CollectorManager}
wrapper for use with
+ * {@link IndexSearcher#search(Query, CollectorManager)}. The wrapping
{@code CollectorManager}
+ * provides no {@link CollectorManager#reduce(Collection)} implementation,
so the wrapped {@code
+ * Collector} needs to do all relevant work while collecting.
+ *
+ * <p>Note: This is only safe to use when {@code IndexSearcher} is created
with no executor (see:
+ * {@link IndexSearcher#IndexSearcher(IndexReader, Executor)}), or the
provided collector is
+ * threadsafe.
+ */
+ static <C extends Collector> CollectorManager<C, ?> wrap(C in) {
+ return new CollectorManager<C, Void>() {
+ @Override
+ public C newCollector() {
+ return in;
Review Comment:
should we assert that this is called once?
--
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...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
