BREAKING: Add cancellation support to IndexSearcher, #922

[paulirwin]

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a change, please open an issue to discuss the change or find an existing issue.

Adds basic cancellation support to IndexSearcher.

Partial #922

Description

This adds optional CancellationToken parameters to all IndexSearcher.Search and SearchAfter methods, which is a breaking change to the API.

If an executor is provided in the constructor for multithreaded search, then the provided CancellationToken is passed to the Task.Wait(CancellationToken) method. If an executor is not provided, then it will call ThrowIfCancellationRequested() in the synchronous case at the evaluation of each leaf reader context.

This will not help cancellation for i.e. single-leaf reader contexts that might take a while to complete (that would require a much larger and messier change to add CancellationToken support to Scorer and ICollector, and benchmarking to ensure we don't hurt performance by doing so), but should at least help with many scenarios that require cancellation.

[NightOwl888]

Repeating this comment from #1119 (comment):

After taking a step back from this and considering that we are working on support for CancellationTokens, I wonder if we should consider removing LimitedConcurrencyLevelTaskScheduler.Shutdown() and always use CancellationToken instead?

If we do keep the LimitedConcurrencyLevelTaskScheduler.Shutdown() method, there are some things to consider:

1. Should we make LimitedConcurrencyLevelTaskScheduler public? The BCL doesn't have one and it is provided in the docs for TaskScheduler. But there is no ShutDown() on that implementation.
2. There are some tests where Shutdown() was called in Lucene that would likely perform better if we use it, since those lines were often commented out in Lucene.NET. However, using CancellationToken could be an alternative way to handle those cases.
   • https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/core/src/test/org/apache/lucene/search/TestBooleanQuery.java#L177-L178
   • https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/core/src/test/org/apache/lucene/search/TestSearcherManager.java#L291-L292
   • https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/suggest/src/test/org/apache/lucene/search/spell/TestSpellChecker.java#L435-L437
   • https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/test-framework/src/java/org/apache/lucene/index/ThreadedIndexingAndSearchingTestCase.java#L645-L646
   • https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/core/src/test/org/apache/lucene/search/TestIndexSearcher.java#L119
   • https://github.com/apache/lucene/blob/releases/lucene-solr/4.8.1/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java#L1570

The upside of using Shutdown() is that it aligns more closely to Lucene.

The upside of using CancellationToken is that it is more fine-grained and can potentially shut down a process quicker, since it will be able to intervene inside of the running task instead of prior to queuing the task.

Allowing a user-defined TaskScheduler still seems like a good idea, but since Microsoft didn't allow them to be cancelled, it seems like we should follow the CancellationToken approach instead of trying to shoehorn a way to cancel a TaskScheduler. Using the same CancellationToken to make LimitedConcurrencyLevelTaskScheduler stop queuing new work is also something that might be worth considering.

[paulirwin]

The LimitedConcurrencyLevelTaskScheduler proposed changes above are outside the scope of this PR, since these changes do not depend on LimitedConcurrencyLevelTaskScheduler (which is only used in tests currently). I agree that it could possibly be a good change to at least improve the tests, so I split that out as #1253.

[paulirwin]

There are several callers of IndexSearcher.Search that will need cancellation support as well to fully realize this, including things like FacetsCollector, DrillSideways, JoinUtil, etc., even things you might not expect like SimpleNaiveBayesClassifier.Train (which seems like it could nicely benefit from cancellation, if you wanted to cancel the training because it was taking too long). I'll add those in a stacked PR after this one is merged. So I have updated this description to remove the word "Fixes" from the issue number, to prevent it from auto-closing once merged.

Meanwhile, I have a few more things to add to polish this up, so it's not quite ready to merge, but this is a good opportunity for the community to chime in if they have any concerns with adding cancellation support to these APIs. This PR has been open for well over a year so I assume there are no concerns, but now is a good time to chime in if so.

[Copilot]

Pull request overview

This PR introduces “best-effort” cancellation to Lucene.NET’s search execution by adding optional CancellationToken parameters to IndexSearcher.Search*/SearchAfter* APIs and wiring the token through both synchronous and executor-based search paths.

Changes:

• Add optional CancellationToken parameters across IndexSearcher search APIs and propagate them through internal search execution.
• Pass cancellation into task scheduling/waiting for multi-threaded search (executor) paths; add leaf-level cancellation checks for synchronous paths.
• Update XML docs and dependent overrides in tests/framework to match the new method signatures.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/Lucene.Net/overview.md	Updates getting-started docs to reference the new Search signature with CancellationToken.
src/Lucene.Net/Support/Threading/TaskSchedulerCompletionService.cs	Adds cancellation support to task submission.
src/Lucene.Net/Search/package.md	Updates package docs to reference new Search overloads including CancellationToken.
src/Lucene.Net/Search/TopFieldDocs.cs	Updates XML docs cref to new IndexSearcher.Search signature.
src/Lucene.Net/Search/TopDocs.cs	Updates XML docs cref to new IndexSearcher.Search signature.
src/Lucene.Net/Search/Sort.cs	Updates XML docs cref to new IndexSearcher.Search signature.
src/Lucene.Net/Search/IndexSearcher.cs	Core implementation: adds cancellation parameters and propagates them through synchronous and executor paths.
src/Lucene.Net/Search/FieldValueHitQueue.cs	Updates XML docs cref to new IndexSearcher.Search signature.
src/Lucene.Net/Search/FieldDoc.cs	Updates XML docs cref to new IndexSearcher.Search signature.
src/Lucene.Net/Search/FieldComparator.cs	Updates XML docs cref references to reflect updated type names and search signatures.
src/Lucene.Net/Search/CollectionTerminatedException.cs	Updates XML docs cref to new low-level search signature including CancellationToken.
src/Lucene.Net.Tests/Search/TestCustomSearcherSort.cs	Updates overridden Search method signatures to include CancellationToken.
src/Lucene.Net.Tests/Search/TestBooleanQuery.cs	Updates overridden protected Search signature to include CancellationToken.
src/Lucene.Net.TestFramework/Search/ShardSearchingTestBase.cs	Updates shard-search overrides/signatures for cancellation support.
src/Lucene.Net.TestFramework/Search/CheckHits.cs	Updates overridden Search method signatures to include CancellationToken.
src/Lucene.Net.TestFramework/Search/AssertingIndexSearcher.cs	Updates overridden protected Search signature to include CancellationToken.
src/Lucene.Net.Misc/Index/Sorter/BlockJoinComparatorSource.cs	Updates docs cref to new SearchAfter signature including CancellationToken.
src/Lucene.Net.Grouping/package.md	(Touched) Documentation line present in grouping docs.

Comments suppressed due to low confidence (3)

src/Lucene.Net/Search/IndexSearcher.cs:1048

• Cancellation won’t propagate correctly in the multi-threaded search path because ExecutionHelper.MoveNext() wraps all exceptions in RuntimeException. If the token is canceled, Wait(cancellationToken) / Result will throw OperationCanceledException, TaskCanceledException, or AggregateException and it will be wrapped, contradicting the new OperationCanceledException contract on the public Search* methods. Add a dedicated catch path that rethrows cancellation (and consider unwrapping AggregateException to preserve OperationCanceledException when tasks are canceled).

                    try
                    {
                        var awaitable = service.Take();
                        awaitable.Wait(cancellationToken);
                        current = awaitable.Result;

                        return true;
                    }
                    catch (Exception e) when (e.IsInterruptedException())
                    {
                        throw new Util.ThreadInterruptedException(e);
                    }
                    catch (Exception e)
                    {
                        throw RuntimeException.Create(e);
                    }

src/Lucene.Net.TestFramework/Search/ShardSearchingTestBase.cs:466

• The cancellation token provided to this Search(...) override is not forwarded to remote shards: the call to SearchNode(...) omits cancellationToken, so cancellation will be ignored for non-local nodes. Pass the token through so the behavior is consistent across all shards.

                        else
                        {
                            shardHits[nodeID] = outerInstance.outerInstance.SearchNode(nodeID, nodeVersions, query, null, numHits, null);
                        }

src/Lucene.Net.TestFramework/Search/ShardSearchingTestBase.cs:530

• Same issue as above: SearchAfter(...) receives a cancellationToken but does not forward it when delegating to SearchNode(...) for remote shards. Pass the token through so cancellation works consistently.

                        else
                        {
                            shardHits[nodeID] = outerInstance.outerInstance.SearchNode(nodeID, nodeVersions, query, null, numHits, shardAfter);
                        }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.