-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(all): tune single Thread into SingleThreadExecutor #5410
feat(all): tune single Thread into SingleThreadExecutor #5410
Conversation
a7968bf
to
dbbb75d
Compare
if (!pool.awaitTermination(60, java.util.concurrent.TimeUnit.SECONDS)) { | ||
pool.shutdownNow(); // Cancel currently executing tasks | ||
// Wait a while for tasks to respond to being cancelled | ||
if (!pool.awaitTermination(60, java.util.concurrent.TimeUnit.SECONDS)) { |
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.
Why there are two waits of 60s, will the 60s time be too long?
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.
60s is the maximum value to be waited until the pool stops, if it doesn't, the pool may have a problem executing the task and needs to be fixed.
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.
copy from java.util.concurrent.ExecutorService
} | ||
|
||
// start json rpc filter process | ||
if (CommonParameter.getInstance().isJsonRpcFilterEnabled()) { | ||
Thread filterProcessThread = new Thread(filterProcessLoop); | ||
filterProcessThread.start(); | ||
filterEs = ExecutorServiceManager.newSingleThreadExecutor(filterEsName, true); |
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.
isDaemon original default value is false
} | ||
logger.info("DPoS task stopped."); | ||
ExecutorServiceManager.shutdownAndAwaitTermination(produceExecutor, name); | ||
logger.info("DPoS task shutdown complete"); |
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.
Can the thread shutdown log be printed in the function shutdownAndAwaitTermination?
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.
Yes, Great!
3e0a838
to
87c1b5d
Compare
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## develop #5410 +/- ##
=============================================
+ Coverage 60.92% 60.94% +0.02%
- Complexity 9231 9242 +11
=============================================
Files 839 840 +1
Lines 50038 50059 +21
Branches 5576 5577 +1
=============================================
+ Hits 30484 30509 +25
+ Misses 17165 17152 -13
- Partials 2389 2398 +9
... and 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
This is #5362 's first PR. close #5409.