-
Notifications
You must be signed in to change notification settings - Fork 26.5k
Fix issue #15698 Distinguish between timeout and task rejection in DefaultFuture #15738
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
base: 3.3
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.3 #15738 +/- ##
============================================
+ Coverage 60.75% 60.77% +0.02%
- Complexity 15 11728 +11713
============================================
Files 1938 1938
Lines 88627 88633 +6
Branches 13381 13380 -1
============================================
+ Hits 53842 53867 +25
+ Misses 29255 29239 -16
+ Partials 5530 5527 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi, I was wondering if you have some time to review my PR. I’d really appreciate your feedback. Thanks a lot! @zrlw |
...bbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/DefaultFuture.java
Show resolved
Hide resolved
70d8bb2 to
a15873e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What is the purpose of the change?
To fix #15698
This pull request improves the handling of Timeout tasks in DefaultFuture when the associated executor rejects the task due to being full or shut down.
Previously, if the executor rejected the timeout task, a RejectedExecutionException was thrown, which could interrupt the processing flow and was not easily consumable by the user. This change:
1. Captures the rejection and generates a proper Response with status SERVER_THREADPOOL_EXHAUSTED_ERROR (100).
2. Sends the response back to the caller via DefaultFuture.received(), ensuring the user can detect that the timeout task was not executed.
3. Removes the future from FUTURES to prevent resource leakage.
This improves both reliability and user visibility of executor rejection scenarios without throwing exceptions.