AKI-692: Refactor Sync manager to use ExecutorService #1141
Conversation
The ExecutorService maintains its own queue making the downloadedBlocks queue redundant.
The downloadedHeaders became unnecessary and was removed.
Also updated the sync request/store/import logic to ensure that the data kept in memory is bounded and well managed.
AlexandraRoatis
force-pushed
the
sync-refactor
branch
from
ee4f863
to
63efe2d
Compare
| pool.shutdownNow(); // Cancel currently executing tasks | ||
| // Wait a while for tasks to respond to being cancelled | ||
| if (!pool.awaitTermination(60, TimeUnit.SECONDS)) { | ||
| log.error("Pool did not terminate"); |
There was a problem hiding this comment.
Should we throw the exception if the kernel fall into this condition? Then we can interrupt the thread.
There was a problem hiding this comment.
Would an exception provide any additional information? I can add a stack trace to the log if you think it's useful?
There was a problem hiding this comment.
To throw the exception is not necessary, it mainly try to interrupt the thread. If the kernel thread stuck, we can dump the process threads info.
| @@ -393,11 +439,32 @@ public long getNetworkBestBlockNumber() { | |||
| } | |||
|
|
|||
| public synchronized void shutdown() { | |||
| if (p2pLog.isDebugEnabled()) { | |||
There was a problem hiding this comment.
minor: the check can be removed
There was a problem hiding this comment.
The check here is actually an optimization to skip the other if statements when debug is disabled. Would you still want it removed?
There was a problem hiding this comment.
I see, because you call the method in the log argument. Just ignore my comment.
Type of change