Fix slow sync #1567
Merged
Fix slow sync #1567
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HoOngEe
force-pushed
the
fix/slow-sync
branch
2 times, most recently
from
14a879e to
2d4328c
Compare
sgkim126
reviewed
May 29, 2019
HoOngEe
force-pushed
the
fix/slow-sync
branch
2 times, most recently
from
67a43b3 to
dab6124
Compare
remagpie
requested changes
May 29, 2019
sync/src/block/extension.rs
Outdated
| // FIXME: handle import errors | ||
| Err(err) => { | ||
| cwarn!(SYNC, "Cannot import header({}): {:?}", header.hash(), err); | ||
| break | ||
| } | ||
| _ => {} | ||
| _ => queued.push(hash), |
There was a problem hiding this comment.
You should handle only queue related error here.
There was a problem hiding this comment.
Now I'm pushing hashes to the queue after importing error occurs.
remagpie
reviewed
May 29, 2019
sync/src/block/downloader/header.rs
Outdated
| .get(&self.pivot.hash) | ||
| .map(Clone::clone) | ||
| .or_else(|| self.client.block_header(&BlockId::Hash(self.pivot.hash))) | ||
| .unwrap(), |
There was a problem hiding this comment.
Why did you use function chaining here?
Using match seems to be more readable for me.
There was a problem hiding this comment.
I changed it to match style.
|
Please fix the commit message. |
sgkim126
requested changes
May 29, 2019
HoOngEe
force-pushed
the
fix/slow-sync
branch
2 times, most recently
from
6b80947 to
2cd87be
Compare
remagpie
previously approved these changes
May 30, 2019
sgkim126
reviewed
May 30, 2019
|
@HoOngEe Please resolve conflicts. |
In current structure, multithreading helps little. Because the verification step should be sequential.
|
I resolved the conficts |
sgkim126
approved these changes
May 31, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Now the total time taken for a node to synchronize with the corgi blocks is about 145 minutes.
Queued cache has introduced distinct from the downloaded cache in
HeaderDownloader.I failed to analyze the correct reason but decreasing
MAX_HEADERS_TO_IMPORTdown to 1,000 helps reducing sync time.And lastly, in the current structure it doesn't help to spawn many threads because the cpu intensive jobs should be sequential. So I decreased it down to
2.The following image shows the cpu time spent during 50,000 block synchronization.
Now the main cpu intensive jobs are all related to EC calculation for verification.