InterruptedException should not be ignored #5515
Conversation
InterruptedException should not be ignored print InterruptedException message and Restore interrupted state
|
Hi @asia-zengtao you should create issue first, This issue describes the problem and explains why. then submit a PR to fixed, you can see this docmuent how the PR related issue. |
|
Welcome ! |
|
InterruptedException is a key approach to enhance program responsiveness... As this article points out, different ways of handling are required according to specific use cases. Unfortunately, most interrupted exceptions in the codebase are simply ignored, leaving a major place to improve. Glad to see that you point this issue out and attempt to improve. |
There was a problem hiding this comment.
Hi @asia-zengtao ,
Great work and welcome to the RocketMQ community. Hope you could keep making our codebase better.
| BrokerController.LOG.warn("shutdown ScheduledExecutorService was Interrupted! ", ignore); | ||
| Thread.currentThread().interrupt(); |
There was a problem hiding this comment.
https://github.com/apache/rocketmq/actions/runs/3449152754/jobs/5767526841
Error: /home/runner/work/rocketmq/rocketmq/broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java:1427:9: File contains tab characters (this is the first instance). [FileTabCharacter]
Error: /home/runner/work/rocketmq/rocketmq/broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java:1427:17: 'catch' child has incorrect indentation level 16, expected level should be 12. [Indentation]
Here has some problems need to fix in CI.
lizhanhui
dismissed stale reviews from hzh0425 and tsunghanjacktsai
via
3abdf0a
Codecov Report
@@ Coverage Diff @@
## develop #5515 +/- ##
=============================================
- Coverage 42.92% 42.41% -0.52%
+ Complexity 8069 7930 -139
=============================================
Files 1029 1020 -9
Lines 72662 71145 -1517
Branches 9604 9381 -223
=============================================
- Hits 31191 30176 -1015
+ Misses 37527 37152 -375
+ Partials 3944 3817 -127
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
* InterruptedException should not be ignored InterruptedException should not be ignored print InterruptedException message and Restore interrupted state * Fix checkstyle issue Co-authored-by: Li Zhanhui <lizhanhui@gmail.com>
InterruptedException should not be ignored
print InterruptedException message and Restore interrupted state
Make sure set the target branch to
developWhat is the purpose of the change
InterruptedException should not be ignored
Brief changelog
print InterruptedException message and Restore interrupted state
Verifying this change
try {
scheduledExecutorService.awaitTermination(5000, TimeUnit.MILLISECONDS);
} catch (InterruptedException ignore) {
BrokerController.LOG.warn("shutdown ScheduledExecutorService was Interrupted! ", ignore);
Thread.currentThread().interrupt();
}
Follow this checklist to help us incorporate your contribution quickly and easily. Notice,
it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.[ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyleto make sure basic checks pass. Runmvn clean install -DskipITsto make sure unit-test pass. Runmvn clean test-compile failsafe:integration-testto make sure integration-test pass.