AKI-692: Refactor Sync manager to use ExecutorService #1141
AKI-692: Refactor Sync manager to use ExecutorService #1141AlexandraRoatis merged 6 commits intomasterfrom
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.
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.
| } | ||
|
|
||
| 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.
AionJayT
left a comment
There was a problem hiding this comment.
LGTM, left two minor comments
Type of change