apache / apache/druid

Fail tests whenever there is an uncaught exception in any thread in Druid

Open
#8,764 2 comments 0 reactions 0 assignees View on GitHub
Area - Testing Contributions Welcome
Dominant language
Java
Stars
14.1k
Forks
3.8k
Avg merge
2d 58m
Merged PRs (30d)
233

Description

This would automatically protect against bugs like https://github.com/code-review-checklists/java-concurrency#concurrent-assert, and, I think, in general, it's a saner way to keep the database in the state that there are no uncaught exceptions happening in the system which cause threads to die. At best, it creates some thread churn (which might be a performance issue), at worst, it masks some problems, [grey failures](https://blog.acolyer.org/2017/06/15/gray-failure-the-achilles-heel-of-cloud-scale-systems/).

This would require some change in Druid's default `ThreadFactory` used for various executors: it needs to set up a special Druid's `UncaughtExceptionHandler` which logs an error in production, but has a flag which allows forwarding the exception to test framework and ultimately fail the test. I don't know exactly how to do this, but this is definitely possible; e. g. it's done in [this class](https://github.com/awaitility/awaitility/blob/8e4c22315ce987584931418e695b96c1c7d49008/awaitility/src/main/java/org/awaitility/core/ConditionAwaiter.java#L33) in Awaitility library. Note that just rethrowing an exception and also using the Awaitility library itself (setting `Awaitility.dontCatchUncaughtExceptions(false)`) would *not* do the trick, because Awaitility [only sets a custom exception handler for threads which don't have it yet](https://github.com/awaitility/awaitility/issues/119#issuecomment-544141814). We would need to do this, too, but it's not enough because we already have threads with a custom `UncaughtExceptionHandler`.

Contributor guide

Open the contributing guide

Research direction

Start by tracing Druid's default ThreadFactory and the executors that use it, then review how existing custom UncaughtExceptionHandlers interact with it. Compare the proposed behavior with Awaitility's ConditionAwaiter and determine how a test-only flag could forward uncaught exceptions to the test framework while production logs them. Done means uncaught exceptions in any relevant test thread reliably fail the test without breaking production handling.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, testing
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.