Reusing a TotalHitCountCollectorManager after a failed search silently undercounts (0 in the repro) or hangs

With intra-segment partitions, TotalHitCountCollectorManager lets the first partition of each segment decide whether the whole segment is counted from index statistics, and stores that decision in a map inside the manager, keyed by the segment only. The map is cleared only in reduce(), and a search that fails during collection never calls reduce(). So the next search with the same manager on the same reader finds the old decisions: every segment marked "already counted" contributes 0 hits, with no error (0 instead of 2 in the repro; count 0 next to TopFieldDocs.totalHits 1000 in a stock-components check). That search's own reduce() clears the map, so the one after it is right again. Every release from 10.0.0 to 10.6.0 and main is affected.

1. The shared decision. TotalHitCountCollectorManager.java:64-75: "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 without further incrementing hit count". The state is one Map<Object, Future<Boolean>> earlyTerminatedMap (a ConcurrentHashMap) per manager (:75), handed to every partition-aware collector (:77-83).
2. The key never changes. getLeafCollector looks up earlyTerminatedMap.get(context.id()) (:110). IndexReaderContext#id() is a per-context identity object (IndexReaderContext.java:41-44, :53-61), the same on every search of the same reader. Neither the query nor the search is part of the key.
3. The first partition decides. TotalHitCountCollectorManager.java:111-125: the first partition of the segment to reach getLeafCollector (the code's "first thread for a given leaf", not necessarily the one with the lowest doc ids) stores a CompletableFuture and calls TotalHitCountCollector#getLeafCollector, which adds the whole segment's count and throws CollectionTerminatedException when weight.count(context) != -1 (TotalHitCountCollector.java:46-54); the manager completes the future with true (TotalHitCountCollectorManager.java:122-124), else with false (TotalHitCountCollectorManager.java:120).
4. The decision is stored before the search fails. MultiCollector#getLeafCollector asks every sub-collector before any collect() (MultiCollector.java:123-143), so the count collector has stored true before the application's collector throws.
5. Clean-up only on success. reduce() is the only place the map is cleared (TotalHitCountCollectorManager.java:86-93, after the comment "Make the same collector manager instance reusable across multiple searches"). IndexSearcher calls reduce after taskExecutor.invokeAll(...) (IndexSearcher.java:788-789), and when a task throws, invokeAll rethrows (TaskExecutor.java:118-135), so a search that fails during collection never gets there.
6. The next search reads the stale note. A stored true makes every partition of that segment throw CollectionTerminatedException (TotalHitCountCollectorManager.java:130-134), the first partition of the new search included. IndexSearcher#searchLeaf treats that as "there is no doc of interest in this reader context" (IndexSearcher.java:835-841). The segment adds 0.
7. Until a search completes. That search reaches reduce(), which clears the map, so the following search is right. A search that also fails before reduce() leaves stale entries for the next one.
8. The hang variant. If the first search failed inside the first partition's Weight#count, the future is never completed and the next search waits forever in earlyTerminated.get() (TotalHitCountCollectorManager.java:131), even single-threaded. That pending future is LFV-045's root cause; the stale map is what carries it into the next search.

Not affected: IndexSearcher#count builds a new manager per call (IndexSearcher.java:525); the IndexSearcher#setTimeout timeout is caught in searchLeaf (:856-857), so reduce() still runs; without partitions the manager hands out plain, stateless collectors (TotalHitCountCollectorManager.java:79-82).

• Opt-in. Partitions are off by default (IndexSearcher.java:317-324); you enable them by overriding slices and calling slices(leaves, maxDocs, maxSegments, true). Lucene's own randomized tests do this often (Concurrency.INTRA_SEGMENT, LuceneTestCaseParent.java:1144-1149).
• Manager reuse. The code says reuse is supported (TotalHitCountCollectorManager.java:87-89); most callers, including IndexSearcher#count, still create a new manager per search.
• The failure. Any exception during collection after a segment's first partition took its count from index statistics (MatchAllDocsQuery, a TermQuery on a segment without deletions, ...): another collector in the MultiCollectorManager (the use the class javadoc names), an ExitableDirectoryReader timeout, an IOException. The IndexSearcher#setTimeout timeout does not count: it is caught and reduce() runs.
• What you see. 0 hits for the affected segments, whatever the next query is, and nothing signals it. In the stock-components check the same response held count 0 and TopFieldDocs.totalHits 1000. With a real 4-thread pool, 200 of 200 rounds were wrong. If the failure was inside Weight#count, the next search hangs.
• How long. Until a search with the manager completes: its reduce() clears the map (normally the very next search).
• Timeout reach (read, not executed). An application that wraps a new ExitableDirectoryReader per request gets new segment ids per request, so a stale entry never matches, even with a reused manager. The timeout trigger needs a reader wrapped once with a resettable timeout; the collector trigger has no such limit.
• Workaround. Create a new TotalHitCountCollectorManager per search (a fresh manager after the failure gives the right count).
• Likelihood: rare (registry): all of opt-in partitions, manager reuse and a failure just before on the same reader.

The test sits in org.apache.lucene.search, as Lucene's own tests do, but uses no package-private member: IndexSearcher(IndexReader, Executor), an override of the protected slices(List) calling the public static slices(leaves, 1, 1, true), getSlices(), TotalHitCountCollectorManager(LeafSlice[]), MultiCollectorManager, SimpleCollector, MatchAllDocsQuery. The executor Runnable::run runs everything on the caller in a fixed order. The test uses no randomness; the framework's randomized defaults (codec, query cache) do not change the result: seeds DEADBEEF, C0FFEE, 5EED1234 and 20 derived seeds all fail.

It compiles unchanged against 10.0.0 to 10.6.0 and main; 9.x has no partition API. Add the Apache license header before using it in a PR. The hang variant is not part of this test: it needs a failure inside Weight#count and is LFV-045's cause.

cp $REPRO/TestTotalHitCountCollectorManagerReuse.java $LUCENE/lucene/core/src/test/org/apache/lucene/search/
cd $LUCENE && ./gradlew :lucene:core:test --tests TestTotalHitCountCollectorManagerReuse -Ptests.seed=DEADBEEF -Ptests.asserts=true
# assertions off fails the same way: -Ptests.asserts=false

TestTotalHitCountCollectorManagerReuse > testReuseAfterFailedSearch FAILED
    java.lang.AssertionError: count of the next search with the same manager expected:<2> but was:<0>
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:F5FBB92F36B7FFED]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.apache.lucene.search.TestTotalHitCountCollectorManagerReuse.testReuseAfterFailedSearch(TestTotalHitCountCollectorManagerReuse.java:59)

# the same with -Ptests.asserts=false, on all 13 releases 10.0.0-10.6.0, and with 20 of 20 derived seeds (assertions off)

• Reuse is designed in. TotalHitCountCollectorManager.java:87-89: "Make the same collector manager instance reusable across multiple searches. It isn't a strict requirement but it is generally supported as collector managers normally don't hold state, as opposed to collectors."
• Upstream code reuses a manager. TestTotalHitCountCollector#testBasics runs two searches with one manager on a newSearcher with a random concurrency, which may be intra-segment (TestTotalHitCountCollector.java:44-57). MustNotBooleanQueryBenchmark keeps one manager in a field and reuses it in every benchmark call (:82, :116, :175-178).
• The design decision. In the #13542 review the author, javanna, wrote (2024-07-22): "Because the collector manager holds state, either we clean it in-between searches (reduce is the current place where that can be done, but that may change over time), or we declare that collector managers cannot be re-used across multiple searches (potential breaking change)", and chose reduce. No review comment discusses a search that fails before reduce(). In #15795 (2026) they wrote "Collectors managers may be reused across multiple executions".
• Where reuse is not allowed, Lucene says so. TopScoreDocCollectorManager.java:26 and TopFieldCollectorManager.java:27: "Note that a new collectorManager should be created for each search due to its internal states." Nothing in CollectorManager, IndexSearcher#search(Query, CollectorManager), MultiCollectorManager, CHANGES.txt or MIGRATE.md says a manager becomes unusable after a failed search; MIGRATE.md presents this class as the example of per-segment handling (MIGRATE.md:1668-1676).
• The interface implies the gap. CollectorManager.java:30-32: reduce "is only called after all leaves have been fully collected".
• The failure is ordinary. Another collector's exception or a timeout propagating out of a search is normal; the manager did nothing wrong.

"CollectorManager promises nothing about reuse, the code comment says reuse 'isn't a strict requirement', and nothing promises reuse after a failed search: this is misuse, and a javadoc note is the answer." This was the cross-model refute review's only substantive objection, rated moderate.

Our answer: it is a reasonable reading of the API lifecycle (the cross-model fact check also rates it moderate), and successful reuse alone does not prove a guarantee after a failure. But reuse is designed in and exercised upstream (the comment, the test, the benchmark, the #13542 choice not to forbid reuse); the managers that must not be reused carry a per-search note and this one does not; CollectorManager itself makes a reduce-only reset miss failed searches; and the result is a plausible wrong number, so a caller cannot tell it should drop the manager. What it gets right: no javadoc promises reuse after a failure in so many words, so a documentation answer is possible; it would leave the minimal test failing.

Two code options were built and run in a scratch worktree of 645d4fe14a20. LFV-045 (the hang inside one search when Weight#count throws) shares the class; its minimal fix does not cure this bug, and the combined patch below fixes both.

The map becomes Map<Weight, Map<Object, Future<Boolean>>>; each partition-aware collector picks its search's inner map in setWeight, which IndexSearcher calls before any leaf (IndexSearcher.java:808-815; MultiCollector forwards it, MultiCollector.java:161-163). getLeafCollector is unchanged and reduce() still clears the outer map. These are the same scoping hunks as LFV-045's option C (only the names differ), the per-weight fix the study model-checked.

// Keyed by the search's Weight first, so that the decisions of a failed search (for which
// reduce() is never called) cannot leak into the next search that reuses this manager.
private final Map<Weight, Map<Object, Future<Boolean>>> earlyTerminatedMap =
    new ConcurrentHashMap<>();

// LeafPartitionAwareTotalHitCountCollector
private final Map<Weight, Map<Object, Future<Boolean>>> perSearch;
private Map<Object, Future<Boolean>> earlyTerminatedMap;

@Override
public void setWeight(Weight weight) {
  super.setWeight(weight);
  earlyTerminatedMap = perSearch.computeIfAbsent(weight, _ -> new ConcurrentHashMap<>());
}

Trade-offs

• Lingering entries. A failed search's inner map stays until the next successful reduce() clears the whole map: one inner map per failed search, at most one entry per segment, and its key keeps that search's Weight (and what it references, such as the query) reachable. Repeated failures with no successful search in between add one each.
• The key is the Weight object. Two searches that share one Weight and one manager would share decisions (IndexSearcher#search(Query, CollectorManager) creates a new Weight per call). The map compares keys with equals/hashCode, which Weight does not override, so Lucene's own weights are told apart by identity; a custom Weight that overrides them could make two searches share decisions (not tested; a key compared by identity would avoid it). One manager used by concurrent searches is not made safe (it is not today either: one search's reduce() clears the map while another runs); not tested.
• Needs setWeight. A getLeafCollector call without a prior setWeight works today (TotalHitCountCollector treats a null weight as "no count shortcut", TotalHitCountCollector.java:48) but would hit a null map with this fix; IndexSearcher always calls it (IndexSearcher.java:811).
• Java 22 _. Fine on main; a 10.x backport needs a named lambda parameter.
• Does not fix LFV-045 (the hang within one search); see the combined patch below. No benchmark was run; the change touches only the partitioned path.

What the checks validated

• Gradle in a scratch worktree of 645d4fe14a20, OpenJDK 25.0.3, seed DEADBEEF: stock, the test fails with assertions on and off; with the fix it passes, on and off.
• Closest suites (the test, TestTotalHitCountCollector, TestIndexSearcher, TestTaskExecutor, TestMultiCollectorManager, TestMultiCollector, TestCollectorManager, TestSearchWithThreads, TestTermQuery, TestBooleanQuery, TestLRUQueryCache, TestTotalHits, TestExitableDirectoryReader): 13 classes, 128 tests, 0 failures, 5 skipped, assertions on and off. Most use newSearcher, which picks intra-segment partitions at random; no coverage was measured.
• checkGoogleJavaFormat, ecjLint and forbiddenApis (main and test) pass with the fix and the test.
• An independent fact check re-ran the gradle runs on a clean copy of 645d4fe14a20 (same results) and ran this fix through the tie-break's independent 1,000-document program: all 9 checks right, including the 4-thread pool and the ExitableDirectoryReader timeout path.
• Not run: the whole core module, a benchmark.

Adds completeExceptionally when the first partition's getLeafCollector throws anything but CollectionTerminatedException, and lets a waiter whose first partition failed skip its partition instead of throwing a wrapped copy (+23/-6). Fixes both bugs.

Trade-offs

One PR for two issues; the quiet-waiter part needs its own justification (it relies on the first partition's exception failing the search, which holds in IndexSearcher).

What the checks validated

Gradle, assertions on: this test, LFV-045's minimal test and the same closest suites, 14 classes, 129 tests, 0 failures, 5 skipped. The diff is byte-identical to LFV-045's validated option C.

completeExceptionally only, with or without removing the entry, ends the hang but keeps the stale true decisions.

Trade-offs

This test still returns 0 instead of 2, and the stock-components check still returns 0 instead of 1000 (executed). TLA+ predicted it before code was written: cb_F_fixrm still violates CountCorrect; only the per-search scoping (cb_F_fixpw) passes.

Add the note TopScoreDocCollectorManager carries.

Trade-offs

No code change, but it contradicts the reuse comment, the upstream test and benchmark, and the #13542 choice not to make reuse a breaking change; callers that reuse a manager keep getting a silent wrong count after any failure, and the minimal test keeps failing.

Method: TLA+ model checking (TLC) of the count barrier (CountBarrier.tla, configuration cb_F, invariant CountCorrect, 31 states, 2026-10-22). In the trace, search 1 decides true (counted from index statistics) for the partitioned segment, then fails while scoring another segment, so no future is left pending; search 2 with the same manager sees the stale true, skips every partition of the segment and returns a short count. The same model showed before any code was written that completeExceptionally plus removing the entry is not enough (cb_F_fixrm) and that scoping the decisions to one search is (cb_F_fixpw). A separate configuration (cb_B, 12 states) gives the hang variant.

The modeller wrote JUnit tests on the real code (3 instead of 13 with two segments). An independent verifier, who read the claim but wrote their own test, reproduced it on main, 10.1.0 and 10.5.2 (0 instead of 13, plus the timeout and hang variants, 500 of 500 wrong on a 4-thread pool). The minimal test here is a one-method reduction of that verifier's test.