From 9172cc42472a55a5f991318a081972ee16b7f186 Mon Sep 17 00:00:00 2001 From: Luca Cavanna Date: Thu, 29 Aug 2024 10:53:53 +0200 Subject: [PATCH] Udpate ReadTask to not rely on search(Query, Collector) (#13602) This commit modifies ReadTask to no longer call the deprecated search(Query, Collector). Instead, it creates a collector manager and calls search(Query, CollectorManager). The existing protected createCollector method is removed in favour of createCollectorManager that returns now a CollectorManager in place of a Collector. SearchWithCollectorTask works the same way if "topScoreDoc" is provided as config. Loading of a custom collector will no longer work, and needs to be replaced with loading a collector manager by class name instead. --- lucene/CHANGES.txt | 3 +++ lucene/benchmark/conf/collector-small.alg | 9 ++++---- lucene/benchmark/conf/collector.alg | 9 ++++---- .../benchmark/byTask/tasks/ReadTask.java | 11 ++++----- .../byTask/tasks/SearchWithCollectorTask.java | 23 +++++++++++-------- 5 files changed, 28 insertions(+), 27 deletions(-) diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index 313032edb4d..e956c99289f 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -218,6 +218,9 @@ Changes in Backwards Compatibility Policy * GITHUB#13230: Remove the Kp and Lovins snowball algorithms which are not supported or intended for general use. (Robert Muir) +* GITHUB#13602: SearchWithCollectorTask no longer supports the `collector.class` config parameter to load a custom + collector implementation. `collector.manager.class` allows users to load a collector manager instead. (Luca Cavanna) + Other --------------------- diff --git a/lucene/benchmark/conf/collector-small.alg b/lucene/benchmark/conf/collector-small.alg index e57ee8646b1..7d312d4f243 100644 --- a/lucene/benchmark/conf/collector-small.alg +++ b/lucene/benchmark/conf/collector-small.alg @@ -17,11 +17,10 @@ # ------------------------------------------------------------------------------------- # multi val params are iterated by NewRound's, added to reports, start with column name. -# collector.class can be: -# Fully Qualified Class Name of a Collector with a empty constructor -# topScoreDocOrdered - Creates a TopScoreDocCollector that requires in order docs -# topScoreDocUnordered - Like above, but allows out of order -collector.class=coll:topScoreDoc +# collector.manager.class can be: +# Fully Qualified Class Name of a CollectorManager with a empty constructor +# topScoreDoc - Creates a TopScoreDocCollectorManager +collector.manager.class=coll:topScoreDoc analyzer=org.apache.lucene.analysis.core.WhitespaceAnalyzer directory=FSDirectory diff --git a/lucene/benchmark/conf/collector.alg b/lucene/benchmark/conf/collector.alg index e2843492dca..96873b9402f 100644 --- a/lucene/benchmark/conf/collector.alg +++ b/lucene/benchmark/conf/collector.alg @@ -17,11 +17,10 @@ # ------------------------------------------------------------------------------------- # multi val params are iterated by NewRound's, added to reports, start with column name. -# collector.class can be: -# Fully Qualified Class Name of a Collector with a empty constructor -# topScoreDocOrdered - Creates a TopScoreDocCollector that requires in order docs -# topScoreDocUnordered - Like above, but allows out of order -collector.class=coll:topScoreDoc +# collector.manager.class can be: +# Fully Qualified Class Name of a CollectorManager with a empty constructor +# topScoreDoc - Creates a TopScoreDocCollectorManager +collector.manager.class=coll:topScoreDoc analyzer=org.apache.lucene.analysis.core.WhitespaceAnalyzer directory=FSDirectory diff --git a/lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReadTask.java b/lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReadTask.java index 58cf8e79efa..a9465e5d12f 100644 --- a/lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReadTask.java +++ b/lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/ReadTask.java @@ -24,7 +24,7 @@ import org.apache.lucene.index.DirectoryReader; import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.MultiBits; import org.apache.lucene.index.StoredFields; -import org.apache.lucene.search.Collector; +import org.apache.lucene.search.CollectorManager; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.Query; import org.apache.lucene.search.ScoreDoc; @@ -119,9 +119,7 @@ public abstract class ReadTask extends PerfTask { hits = searcher.search(q, numHits); } } else { - Collector collector = createCollector(); - - searcher.search(q, collector); + searcher.search(q, createCollectorManager()); // hits = collector.topDocs(); } @@ -184,9 +182,8 @@ public abstract class ReadTask extends PerfTask { return res; } - protected Collector createCollector() throws Exception { - return new TopScoreDocCollectorManager(numHits(), withTotalHits() ? Integer.MAX_VALUE : 1) - .newCollector(); + protected CollectorManager createCollectorManager() throws Exception { + return new TopScoreDocCollectorManager(numHits(), withTotalHits() ? Integer.MAX_VALUE : 1); } protected Document retrieveDoc(StoredFields storedFields, int id) throws IOException { diff --git a/lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/SearchWithCollectorTask.java b/lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/SearchWithCollectorTask.java index f41994f48d6..c753ccde65a 100644 --- a/lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/SearchWithCollectorTask.java +++ b/lucene/benchmark/src/java/org/apache/lucene/benchmark/byTask/tasks/SearchWithCollectorTask.java @@ -19,7 +19,7 @@ package org.apache.lucene.benchmark.byTask.tasks; import org.apache.lucene.benchmark.byTask.PerfRunData; import org.apache.lucene.benchmark.byTask.feeds.QueryMaker; import org.apache.lucene.benchmark.byTask.utils.Config; -import org.apache.lucene.search.Collector; +import org.apache.lucene.search.CollectorManager; import org.apache.lucene.search.TopScoreDocCollectorManager; /** Does search w/ a custom collector */ @@ -37,7 +37,11 @@ public class SearchWithCollectorTask extends SearchTask { // check to make sure either the doc is being stored PerfRunData runData = getRunData(); Config config = runData.getConfig(); - clnName = config.get("collector.class", ""); + if (config.get("collector.class", null) != null) { + throw new IllegalArgumentException( + "collector.class is no longer supported as a config parameter, use collector.manager.class instead to provide a CollectorManager class name"); + } + clnName = config.get("collector.manager.class", ""); } @Override @@ -46,18 +50,17 @@ public class SearchWithCollectorTask extends SearchTask { } @Override - protected Collector createCollector() throws Exception { - Collector collector = null; + protected CollectorManager createCollectorManager() throws Exception { + CollectorManager collectorManager; if (clnName.equalsIgnoreCase("topScoreDoc") == true) { - collector = - new TopScoreDocCollectorManager(numHits(), null, Integer.MAX_VALUE, false).newCollector(); + collectorManager = new TopScoreDocCollectorManager(numHits(), Integer.MAX_VALUE); } else if (clnName.length() > 0) { - collector = Class.forName(clnName).asSubclass(Collector.class).getConstructor().newInstance(); - + collectorManager = + Class.forName(clnName).asSubclass(CollectorManager.class).getConstructor().newInstance(); } else { - collector = super.createCollector(); + collectorManager = super.createCollectorManager(); } - return collector; + return collectorManager; } @Override