Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

During concurrent slice searches in IndexSearcher stop other tasks if one throws an Exception #12756

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

quux00
Copy link
Contributor

@quux00 quux00 commented Nov 3, 2023

Description

Since TaskExecutor now waits for all concurrent tasks to finish, even if one throws an Exception and when an exception is thrown, any remaining unscheduled tasks are cancelled, the next step is to notify currently running tasks to exit early. This is done via a a new QueryTimeout implementation, ExceptionBasedQueryTimeout, which holds a volatile boolean of whether any other sibling task threw an exception. If the boolean is true, then the shouldExit method returns true, so that the in progress task exits early.

Closes #12278

@quux00 quux00 force-pushed the concurrent-search/stop-all-on-exception branch from dd0bb0a to 78d2d5a Compare November 3, 2023 19:27
… one throws an Exception.

Since TaskExecutor now waits for all concurrent tasks to finish, even if one throws an
Exception and when an exception is thrown, any remaining unscheduled tasks are cancelled,
the next step is to notify currently running tasks to exit early. This is done via a
a new QueryTimeout implementation, ExceptionBasedQueryTimeout, which holds a volatile
boolean of whether any other sibling task threw an exception. If the boolean is true,
then the shouldExit method returns true, so that the in progress task exits early.

Closes apache#12278
@quux00 quux00 force-pushed the concurrent-search/stop-all-on-exception branch from 78d2d5a to c0e8969 Compare November 3, 2023 19:48
Copy link
Contributor

@stefanvodita stefanvodita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I wish we didn't have to add those methods for the test, but it seems that is a necessary evil. Could you also add an entry to CHANGES.txt?

public void testMultipleSegmentsWithExceptionCausesEarlyTerminationOfRunningTasks() {
// skip this test when only one leaf, since one leaf means one task
// and the TimeLimitingBulkScorer will NOT be added in IndexSearcher
if (reader.leaves().size() <= 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other tests which create their own index. Is it worth doing that just to avoid this case? I'm not sure.

Copy link

github-actions bot commented Jan 8, 2024

This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the dev@lucene.apache.org list. Thank you for your contribution!

@github-actions github-actions bot added the Stale label Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve exception handling for concurrent slice search
2 participants