apache / apache/accumulo

Improve handling of interrupt conditions.

Open
#1,893 7 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Java
Stars
1.2k
Forks
487
Avg merge
4d 5h
Merged PRs (30d)
13

Description

**Is your feature request related to a problem? Please describe.**
We are not consistent on handling interrupts.

**Describe the solution you'd like**
1) Code should not swallow interrupts without at least resetting the interrupt flag - throwing an Exception if possible and appropriate.
2) If an exception cannot be thrown, the caller code should be checking interrupt status and reacting if an interrupt occurs.

**Describe alternatives you've considered**
The situation described below is a concrete examples, but similar issues occur in other places - these could be handled in bulk - or at least if the code is modified for other reason, corrected at that time on a case by case basis.

**Additional context**
When reviewing #1890 - the code in BulkImportCacheCleaner caught the interrupt exception like this:

```
} catch (KeeperException | InterruptedException e) {
// we'll just clean it up again later
log.debug("Error reading bulk import live transactions {}", e);
}
```
The code was modified to reassert the interrupt with:

https://github.com/apache/accumulo/blob/307bef2fe6a8b2b7152eabaf9b16950cff72729a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/BulkImportCacheCleaner.java#L57-L64

The caller code - in TabletServer schedules this task with the following:

https://github.com/apache/accumulo/blob/6af856b38bb57f49418547a8b9c262c66a1fe31e/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java#L778-L780

The call to `scheduleWithFixedDelay` returns a `ScheduledFuture.` that future could be used to attempt to cancel running tasks - or at least prevent new tasks from being scheduled if the interrupt status was checked. The goal would be to provide a more graceful approach to termination when possible (like closing file descriptors) - or at least improve termination responsiveness, The running tasks would probably need to properly handle `mayInterruptIfRunning `

Other considerations:

Some of the exception handling may have improved with the standardization of thread pools in #1808 - and maybe that code could also check for interrupts as an alternate approach.

This task may overlap with #946

Contributor guide

Open the contributing guide

Research direction

Start by reading the interrupt handling in BulkImportCacheCleaner and the scheduling call in TabletServer referenced by the issue. Review the thread-pool changes from #1808 and possible overlap with #946 before surveying similar exception handlers. Done means agreeing on a bounded scope and a consistent, tested approach for interrupt propagation and task termination.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
distributed-systems
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.