gsmiller commented on a change in pull request #142:
URL: https://github.com/apache/lucene/pull/142#discussion_r639646070
##########
File path: lucene/facet/src/java/org/apache/lucene/facet/DrillSidewaysQuery.java
##########
@@ -40,21 +41,65 @@
// TODO change the way DrillSidewaysScorer is used, this query does not work
// with filter caching
class DrillSidewaysQuery extends Query {
+
final Query baseQuery;
- final Collector drillDownCollector;
- final Collector[] drillSidewaysCollectors;
+
+ // The caller must either directly provide FacetsCollectors for the drill
down and sideways
+ // collecting, or provide FacetsCollectorManagers used to create the
FacetsCollectors. If
+ // this query will be executed concurrently, FacetsCollectorManagers should
be used to ensure
+ // multiple threads aren't collecting into the same FacetsCollector.
+ final FacetsCollector drillDownCollector;
+ final FacetsCollector[] drillSidewaysCollectors;
+ final FacetsCollectorManager drillDownCollectorManager;
+ final FacetsCollectorManager[] drillSidewaysCollectorManagers;
+ final List<FacetsCollector> managedDrillDownCollectors;
+ final List<FacetsCollector[]> managedDrillSidewaysCollectors;
+
final Query[] drillDownQueries;
+
final boolean scoreSubDocsAtOnce;
+ /**
+ * Construct a new {@code DrillSidewaysQuery} that will directly use the
provided {@link
+ * FacetsCollector}s. Use this if you're certain that the query will not be
executed concurrently.
Review comment:
Since this class is package-private and I think it makes sense to
consolidate it to always use `FacetsCollectorManager` (as per your suggestion),
I think I'll add some detailed documentation on the `DrillSideways` class to
describe the trade-off of providing an `ExecutorService` or not. This is what
triggers the "concurrent drill sideways" implementation. The short version
though is that the concurrent approach to DS will do lots of duplicate work but
may be slightly faster, where as the "sequential" version doesn't do any
duplicate query processing/computation. It gets extra confusing because both
the "sequential" and "concurrent" approaches can still work with concurrent
query execution if using a `CollectorManager` + `IndexSearcher` with an
`Executor`. It's a little tricky to describe but I'll take a shot at it.
--
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.
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
