feat(all):name thread pools #5425
Conversation
halibobo1205
requested review from
317787106,
tomatoishealthy,
xxo1shine and
jwrct
| Executors.newSingleThreadScheduledExecutor( | ||
| new ThreadFactoryBuilder().setNameFormat("db-stats-thread-%d").build()); | ||
|
|
||
| private final String esName = "db-stats"; |
There was a problem hiding this comment.
For singleton, there should be no need.
| @@ -983,11 +983,13 @@ public Pair<Boolean, byte[]> execute(byte[] rawData) { | |||
| public static class BatchValidateSign extends PrecompiledContract { | |||
|
|
|||
| private static final ExecutorService workers; | |||
| private static final String workersName = "validate-sign-contract"; | |||
There was a problem hiding this comment.
Should this managed by ExecutorServiceManager?
There was a problem hiding this comment.
Yes, do you have any good ideas?
There was a problem hiding this comment.
So we can just move the excutors name and the construction into ExecutorServiceManager, then the lifecycle and all the executors is controlled in ExecutorServiceManager , a certain executor shall be obtained by using a name(may be obtained by spring injection).
There was a problem hiding this comment.
Thread pool shutdown is better controlled by the service, not the manager but this one is a special case, so maybe it can be done this way.
halibobo1205
force-pushed
the
feat/thread_pool_opt
branch
from
04e11f8
to
ecb622f
Compare
| @@ -102,7 +103,7 @@ private void check(PeerConnection peer, TransactionsMessage msg) throws P2pExcep | |||
| private void handleSmartContract() { | |||
| smartContractExecutor.scheduleWithFixedDelay(() -> { | |||
| try { | |||
| while (queue.size() < MAX_SMART_CONTRACT_SUBMIT_SIZE) { | |||
| while (queue.size() < MAX_SMART_CONTRACT_SUBMIT_SIZE && smartContractQueue.size() > 0) { | |||
There was a problem hiding this comment.
Why add this logic, if the queue has no elements, it will enter an infinite loop.
There was a problem hiding this comment.
TrxEvent event = smartContractQueue.take();
Retrieves and removes the head of this queue, waiting if necessary until an element becomes available.
| @@ -102,7 +103,7 @@ private void check(PeerConnection peer, TransactionsMessage msg) throws P2pExcep | |||
| private void handleSmartContract() { | |||
| smartContractExecutor.scheduleWithFixedDelay(() -> { | |||
| try { | |||
| while (queue.size() < MAX_SMART_CONTRACT_SUBMIT_SIZE) { | |||
| while (queue.size() < MAX_SMART_CONTRACT_SUBMIT_SIZE && smartContractQueue.size() > 0) { | |||
There was a problem hiding this comment.
Try not to change the original logic, if it is a bug, it is recommended to submit a pr separately.
There was a problem hiding this comment.
@wubin01 , As explained above, smartContractExecutor.shutdown() -> ExecutorServiceManager.shutdownAndAwaitTermination(smartContractExecutor, smartEsName)
if queue is empty, will block 60s
| @@ -117,18 +116,6 @@ public void init() { | |||
| exitThread.start(); | |||
| } | |||
There was a problem hiding this comment.
Why not adjust exitThread to SingleThreadExecutor?
There was a problem hiding this comment.
exitThread will only run once, when it receives the exit signal LockSupport.unpark(exitThread) then System.exit(1), otherwise it blocks in here LockSupport.park(), unlike other single-thread that has while(true) logic.
halibobo1205
force-pushed
the
feat/thread_pool_opt
branch
from
ecb622f
to
d0b03ab
Compare
halibobo1205
force-pushed
the
feat/thread_pool_opt
branch
from
d0b03ab
to
c75f33b
Compare
close #5422 .