Monitor's ParallelMatcher and PartitionMatcher lose every match when one query throws an unchecked exception, and ParallelMatcher can hang

Monitor.match runs every query the presearcher selects through a CandidateMatcher. The default serial path wraps each query in catch (Exception e) { matcher.reportError(id, e); }, so a query that throws is reported in getErrors() and the other matches are returned. The package docs offer PartitionMatcher and ParallelMatcher as wrappers "to increase performance in low-concurrency systems". Their workers catch only IOException. An unchecked exception, such as the IllegalArgumentException an IntPoint query throws on a field indexed as LongPoint, ends the worker; doFinish turns it into RuntimeException("Interrupted during match"), and Monitor.match throws and returns nothing, with a message that names neither the query nor the cause. ParallelMatcher also feeds its workers through a bounded queue of 1,024 tasks with an untimed put: once every worker has died and the queue fills, Monitor.match blocks forever, and each interrupt frees only one blocked put (the recorded runs needed 77 to 79). The code came with the module (LUCENE-8766, 2019); all 58 releases from 8.2.0 to 10.6.0 and main are affected.

1. Serial path, the contract. Monitor.match feeds each selected query through StandardQueryCollector.matchQuery and then calls matcher.finish(...) (Monitor.java:218-226). The collector catches Exception and calls matcher.reportError(id, e) (Monitor.java:297-302). CandidateMatcher.reportError(String, Exception) is "Called by the Monitor if running a query throws an Exception" (CandidateMatcher.java:97-100); getErrors() returns a Map<String, Exception>.
2. PartitionMatcher. matchQuery only queues the task (PartitionMatcher.java:70-73), so the collector's catch never sees a failure. doFinish splits the tasks into threads partitions and runs them with executor.invokeAll (:81-90). Each worker wraps each query in catch (IOException e) only (:116-121). An unchecked exception escapes call(), future.get() (:91) throws ExecutionException, and that becomes RuntimeException("Interrupted during match", e) (:99-100). Monitor.match propagates it: no result at all.
3. ParallelMatcher. The constructor submits threads workers (ParallelMatcher.java:69-72; the two-argument factory uses availableProcessors(), :180-184). matchQuery puts each task on a LinkedBlockingQueue<>(1024) with an untimed put (:49, :76-84). Each worker loops on queue.take() and catches only IOException (:124-138, catch at :130). doFinish puts one END per worker (:94-96) and calls future.get() (:99), wrapping ExecutionException in RuntimeException("Interrupted during match") (:110-111).
4. The hang. When every worker has died, nobody takes from the queue. After 1,024 queued tasks the next put (:80) blocks with no timeout (or, if only the END markers overflow it, doFinish's put(END) at :95 does). An interrupt turns that one put into IOException("Interrupted during match") (:81-82), which the collector records as that query's error (Monitor.java:300-301), and the next put blocks again. With 1 thread, 1 failing query first and 1,100 good ones: 1,100 - 1,024 = 76 blocked puts plus the final put(END) = 77 interrupts, as observed.
5. Precondition of the hang. With 2 threads, 2 failing queries plus 1,100 good ones hang; with 1 failing query (one worker survives and empties the queue) or with only 100 good ones, Monitor.match throws without hanging. QueryDecomposer splits a plain disjunction (no MUST or FILTER clause, minimumNumberShouldMatch at most 1) into one candidate per distinct SHOULD clause; a conjunction or minimumNumberShouldMatch > 1 stays whole, and equal clauses collapse into one candidate (QueryDecomposer.java:77-103). So one registered disjunction with threads distinct failing clauses is enough.
6. Only the exception type matters. Both wrappers report a checked IOException from a query per query and return the other matches; the same failure thrown as UncheckedIOException loses the batch (tie-break probe).

• Who is hit. Applications that match with ParallelMatcher or PartitionMatcher from the optional lucene-monitor module. The default serial matchers are not affected.
• Ordinary triggers, all accepted by Monitor.register and all reported per query by the serial matcher: a point query whose width does not match the indexed field (it depends on the documents, so it matters where documents and queries come from independent producers); a query that passes each BooleanQuery builder but exceeds IndexSearcher's clause limit at rewrite (two 601-clause disjunctions in a conjunction give TooManyNestedClauses; one classic QueryParser string with default settings is enough); a custom Query that throws a RuntimeException; an UncheckedIOException.
• Lost batch. One failing query makes the whole Monitor.match call throw, at any thread count, with assertions on or off: every match of every document in the batch is lost, and "Interrupted during match" hides the cause (it is only in the cause chain) and the failing query id (nothing names it). A term-less point query is selected for every document, so it breaks every match whose document has the field with the other point width.
• Hang (ParallelMatcher only). Needs at least threads failing candidates in one batch (default threads = availableProcessors()) and then roughly 1,024 or more candidates still to queue. The matching thread then blocks forever, and one interrupt (what Future.cancel(true) sends) does not free it.
• Reach. Rare. Code search found no use of the two wrappers in Elasticsearch, OpenSearch or Solr (Solr's open monitor-integration PR uses its own matcher); private deployments are not visible; nothing has been reported in seven years.
• No data damage. The call fails loudly or blocks; the query index is not changed.

Public API only (the test sits in org.apache.lucene.monitor for locality; nothing package-private is used), one method, no randomness of its own; a second seed fails the same way. An executor is part of the API under test (PartitionMatcher always runs its partitions on one); the two threads are the test's choice. The pool is built with Lucene's NamedThreadFactory, as the forbidden-API check requires (it rejects Executors.newFixedThreadPool(int)). The serial matcher returns exactly what the test asserts (1 match and the error of int), and with the fix PartitionMatcher does too.

It covers PartitionMatcher only. ParallelMatcher has the identical catch, and the same fix covers it; its lost batch and its hang were shown by a separate probe (serial control, PartitionMatcher(4), ParallelMatcher(4), and the 4-thread hang with one 4-clause failing query plus 1,100 good ones, bounded at 20 s with an interrupt loop), which is evidence, not a regression test.

Suggested regression test: this same test, as its own class or as one method in TestPartitionMatcher. With the fix it passes, and so does :lucene:monitor:check (format check, ecj lint, forbidden APIs and the whole suite).

# main 645d4fe14a20, gradle, seed DEADBEEF, -Ptests.asserts=true (the same with false); excerpt, the cut marked
TestPartitionMatcherUncheckedQueryError > testUncheckedQueryErrorIsReportedPerQuery FAILED
    java.lang.AssertionError: expected 1 match and an error for query "int", but Monitor.match threw java.lang.RuntimeException: Interrupted during match
        at __randomizedtesting.SeedInfo.seed([DEADBEEF:E125866324F54F3E]:0)
        at org.apache.lucene.monitor.TestPartitionMatcherUncheckedQueryError.testUncheckedQueryErrorIsReportedPerQuery(TestPartitionMatcherUncheckedQueryError.java:45)

        Caused by:
        java.lang.RuntimeException: Interrupted during match
            at org.apache.lucene.monitor.PartitionMatcher.doFinish(PartitionMatcher.java:100)
            at org.apache.lucene.monitor.CandidateMatcher.finish(CandidateMatcher.java:106)
            at org.apache.lucene.monitor.Monitor.match(Monitor.java:225)
            at org.apache.lucene.monitor.Monitor.match(Monitor.java:242)
            at org.apache.lucene.monitor.TestPartitionMatcherUncheckedQueryError.testUncheckedQueryErrorIsReportedPerQuery(TestPartitionMatcherUncheckedQueryError.java:42)

            Caused by:
            java.util.concurrent.ExecutionException: java.lang.IllegalArgumentException: field="num" was indexed with bytesPerDim=8 but this query has bytesPerDim=4
                at java.base/java.util.concurrent.FutureTask.report(FutureTask.java:124)
                at java.base/java.util.concurrent.FutureTask.get(FutureTask.java:193)
                at org.apache.lucene.monitor.PartitionMatcher.doFinish(PartitionMatcher.java:91)
                ... 4 more

                Caused by:
                java.lang.IllegalArgumentException: field="num" was indexed with bytesPerDim=8 but this query has bytesPerDim=4
                    at org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.search.PointRangeQuery.checkValidPointValues(PointRangeQuery.java:641)
                    at org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.search.PointRangeQuery.rewrite(PointRangeQuery.java:548)
                    at org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.search.ConstantScoreQuery.rewrite(ConstantScoreQuery.java:44)
                    at org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.search.IndexSearcher.rewrite(IndexSearcher.java:873)
                    at org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.search.IndexSearcher.rewrite(IndexSearcher.java:887)
                    at org.apache.lucene.core@11.0.0-SNAPSHOT/org.apache.lucene.search.IndexSearcher.search(IndexSearcher.java:753)
                    at org.apache.lucene.monitor.CollectingMatcher.matchQuery(CollectingMatcher.java:43)
                    at org.apache.lucene.monitor.PartitionMatcher$MatcherWorker.call(PartitionMatcher.java:118)
                    at org.apache.lucene.monitor.PartitionMatcher$MatcherWorker.call(PartitionMatcher.java:104)
                    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:328)
                    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1090)
                    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:614)
                    at java.base/java.lang.Thread.run(Thread.java:1474)

[... gradle's copy of the same trace and the test-environment notes cut ...]

1 test completed, 1 failed

# the same test with the fix (both workers catch Exception), asserts on and off: :lucene:monitor:test (SUCCESS): 1 test

# ParallelMatcher hang, supporting probe on main, assertions off (not the minimal test; one frame cut, marked)
ParallelMatcher(1 thread, 1 failing + 1100 good) = BUG: HANG, Monitor.match still blocked after 30 s in
  java.util.concurrent.LinkedBlockingQueue.put <- org.apache.lucene.monitor.ParallelMatcher.matchQuery
  <- org.apache.lucene.monitor.Monitor$StandardQueryCollector.matchQuery <- [...]; stopped after 77 interrupts

• The serial path reports any Exception per query. The collector that drives every matcher catches Exception (Monitor.java:300); reportError takes an Exception and its javadoc says it is "Called by the Monitor if running a query throws an Exception" (CandidateMatcher.java:97); getErrors() is a Map<String, Exception>. No javadoc states this as a guarantee of the wrappers; it is the behaviour of the code that drives them. All of this dates from the module's first commit.
• The wrappers report per query too, but for one exception type only. Both workers catch and report an IOException per query (PartitionMatcher.java:119-120, ParallelMatcher.java:130-131), and that works; the same failure as UncheckedIOException loses the batch.
• Documented as a performance option. The package docs (package-info.java:50-51) and the class javadocs discuss only performance trade-offs; nothing says one failing query aborts the batch or that the caller may block.
• The module's author stated the intent in the predecessor project. On Luwak #3 (2014), about a query that threw an unchecked exception, they wrote that "this should be dealt with by the error-handling code now ... it will add an error to the final DocumentMatches object". Luwak #1 ("Aborting search after single query fail") led to the per-query error handling. The concurrent matchers came later and never got the same catch.
• The message shows the path was not foreseen: "Interrupted during match" when nothing was interrupted.
• Tests. The shared concurrent-matcher tests cover only the no-error path; the two error tests (TestMonitorErrorHandling.testMonitorErrors, TestHighlightingMatcher.testQueryErrors) use serial matchers.

The strongest case for "not a bug" (the devil's-advocate verifier and the cross-model reviews): CandidateMatcher.matchQuery declares only @throws IOException on IO errors, and both upstream error tests use a query that throws an IOException, so perhaps only I/O errors are meant to be isolated and a RuntimeException is a programming error that should propagate; core also throws IllegalArgumentException for a point-width mismatch on purpose.

Why it fails: throws IOException is the checked-exception declaration Java requires, and the code that calls every matchQuery, the Monitor's collector, catches Exception. Nobody disputes that core throws; the question is how the monitor reports it, and its serial path reports it per query. Even on the narrowest reading (let unchecked exceptions propagate), the misleading message stays a defect, and a matching thread that blocks forever is no defensible outcome. Verdict: not intended.

Option A was validated with gradle in a scratch worktree of 645d4fe14a20; options B and C were not built.

In ParallelMatcher.MatcherWorker.call and PartitionMatcher.MatcherWorker.call, catch Exception instead of IOException and report it per query, as Monitor's collector does. In PartitionMatcher the report goes into the outer matcher's HashMap, which several workers can write at once, so it is wrapped in synchronized (PartitionMatcher.this); the now unused java.io.IOException import goes (the format check rejects it otherwise).

// ParallelMatcher.MatcherWorker.call
          } catch (Exception e) {   // was: IOException
            matcher.reportError(task.id, e);
          }
// PartitionMatcher.MatcherWorker.call
        } catch (Exception e) {     // was: IOException
          synchronized (PartitionMatcher.this) {
            PartitionMatcher.this.reportError(task.queryId, e);
          }
        }
// and drop PartitionMatcher's now unused import java.io.IOException

Trade-offs

• Same behaviour as the serial path: the failing query is in getErrors(), all other matches are returned. The changed code (the wider catch and the lock) is reached only when a query throws; the cost was not measured.
• Not covered: an Error (for example an AssertionError under -ea, or StackOverflowError) still ends a worker. For PartitionMatcher that matches the serial path, which does not catch Error either; for ParallelMatcher enough such workers can still produce the hang (option B).
• The lock guards a race that exists today on the IOException path; it has no repro and is not claimed separately.

What the checks validated

• Gradle, scratch worktree of 645d4fe14a20 with the repro test added, seed DEADBEEF. Stock: the repro fails with -Ptests.asserts=true and false, and the whole :lucene:monitor:test suite runs 182 tests (33 classes) with exactly 1 failure, the repro, and 1 skipped. With the fix: the repro passes with assertions on and off; the whole suite passes (182 tests, 0 failures, 1 skipped); checkGoogleJavaFormat and ecjLintMain of the monitor module pass.
• With the revised test (its pool built with NamedThreadFactory): the repro passes with assertions on and off, and :lucene:monitor:check passes (format check, ecj lint and forbidden APIs of main and test sources, and the suite: 182 tests, 0 failures, 1 skipped).
• The same catch change, shadowed into the stock jars, also passed the independent verifier's 7 tests, the devil's-advocate probe (4 of 4: its 4-thread hang test then returns) and the tie-break probe (7 of 7: the 1-thread case returns 1,100 matches plus the error).

Let ParallelMatcher.MatcherWorker.call catch Throwable around one task, remember it and keep emptying the queue until END before rethrowing it; or make matchQuery and doFinish use queue.offer(task, timeout) in a loop that gives up once every worker future is done.

Trade-offs

Removes the hang for Errors too, at the cost of more code; the timed variant polls. Not built.

Report into each worker's own sub-matcher (matcher.reportError) and copy matches.getErrors() in doFinish, as ParallelMatcher.doFinish already does (ParallelMatcher.java:105-107). This avoids the shared map and the lock.

Trade-offs

Two more lines in doFinish. Not built.

For an ExecutionException, say that a query failed (and keep the cause) instead of "Interrupted during match".

Trade-offs

Cosmetic on its own; with A the path is reached only by an Error.

Non-formal. A hunt unit on the monitor module first ran exhaustive presearcher checks (clean), then read the matchers it named. The workers' IOException-only catch, next to the serial collector's catch (Exception e), stood out; the finder wrote a JUnit with a stock trigger (a nested BooleanQuery over IndexSearcher's clause limit), which failed on the stock main jars with assertions on and off, while the serial control passed. Independent verifiers then found the simpler point-width trigger and pinned down the hang's precondition.