javanna commented on code in PR #13542: URL: https://github.com/apache/lucene/pull/13542#discussion_r1686922601
########## lucene/core/src/java/org/apache/lucene/search/TotalHitCountCollectorManager.java: ########## @@ -28,17 +31,77 @@ */ public class TotalHitCountCollectorManager implements CollectorManager<TotalHitCountCollector, Integer> { + + /** + * Internal state shared across the different collectors that this collector manager creates. It + * tracks leaves seen as an argument of {@link Collector#getLeafCollector(LeafReaderContext)} + * calls, to ensure correctness: if the first partition of a segment early terminates, count has + * been already retrieved for the entire segment hence subsequent partitions of the same segment + * should also early terminate. If the first partition of a segment computes hit counts, + * subsequent partitions of the same segment should do the same, to prevent their counts from + * being retrieve from {@link LRUQueryCache} (which returns counts for the entire segment) + */ + private final Map<LeafReaderContext, Boolean> seenContexts = new HashMap<>(); Review Comment: right, those are good points that I wonder as well about. We currently synchronize on the map, it would be enough to synchronize at the context level. I tried that but the code was quite hard to follow as I was synchronizing on a method argument. -- 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