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

fail fast in indexParallel() on ExecutionException #4705

Open
vladak opened this issue Jan 7, 2025 · 2 comments
Open

fail fast in indexParallel() on ExecutionException #4705

vladak opened this issue Jan 7, 2025 · 2 comments

Comments

@vladak
Copy link
Member

vladak commented Jan 7, 2025

If an index task fails in indexParallel() with InterruptedException or ExecutionException exception, it will be detected only once the cycle gets to the respective future when traversing the list and calls future.get():

for (var future : futures) {
IndexFileWork work = future.get();
bySuccess.computeIfAbsent(work.ret, key -> new ArrayList<>()).add(work);
}

Ideally, this should fail fast, i.e. the first future from the list which exhibited the exception should terminate the processing for given index database.

Came across this when working on PR #4706.

@vladak
Copy link
Member Author

vladak commented Jan 7, 2025

Calling parallelizer.getIndexWorkExecutor().shutdownNow() in the catch block below the cycle is probably not feasible as it might impact indexing of other projects. On the other hand if the indexer fails with OOM (ExecutionException) then all bets are off.

@vladak
Copy link
Member Author

vladak commented Jan 8, 2025

Another idea is to use ExecutorCompletionService so that it is possible to retrieve the already complete Future objects. Then the rest of the futures submitted in indexParallel() can be cancel()ed in case of ExecutionException.

The only caveat is that multiple IndexDatabase#update()s can run in parallel (from Indexer#doIndexerExecution()) and there is a single ExecutorCompletionService (instantiated in and retrieved via IndexParallelizer so that all indexing can be capped by thread pool size) has only single queue of futures so it can happen that futures which belong to distinct IndexDatabase could be returned. Hence, the BlockingQueue and ExecutorCompletionService would need to be extended/reimplemented (plus related interfaces) to provide filtering in take(tag) based on a tag.

Also, ExecutorCompletionService does not seem to have any of the shutdown methods like ExecutionService so that would need changes in IndexParallelizer#bounceIndexWorkExecutor().

@vladak vladak changed the title fail fast in indexParallel() fail fast in indexParallel() on ExecutionException Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant