Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fix ancient blocks sync #9531

Merged
merged 47 commits into from
Oct 9, 2018
Merged
Changes from 14 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
5f38aa8
Log block set in block_sync for easier debugging
ascjones Sep 7, 2018
5f219a4
logging macros
ascjones Sep 10, 2018
8d7f742
Match no args in sync logging macros
ascjones Sep 10, 2018
8f379ee
Add QueueFull error
ascjones Sep 10, 2018
f5d244e
Only allow importing headers if the first matches requested
ascjones Sep 10, 2018
f77c503
WIP
ascjones Sep 10, 2018
0f3adb3
Test for chain head gaps and log
ascjones Sep 11, 2018
ad7bb2e
Calc distance even with 2 heads
ascjones Sep 11, 2018
56747ef
Revert previous commits, preparing simple fix
ascjones Sep 11, 2018
0eb8655
Reject headers with no gaps when ChainHead
ascjones Sep 11, 2018
72783a2
Reset block sync download when queue full
ascjones Sep 11, 2018
6f7c3c2
Simplify check for subchain heads
ascjones Sep 11, 2018
b4033f3
Add comment to explain subchain heads filter
ascjones Sep 11, 2018
7d1bc76
Fix is_subchain_heads check and comment
ascjones Sep 11, 2018
9588e5a
Prevent premature round completion after restart
ascjones Sep 12, 2018
9b31755
Reset stale old blocks request after queue full
ascjones Sep 13, 2018
8895cc6
Revert "Reject headers with no gaps when ChainHead"
ascjones Sep 13, 2018
6e4a560
Add BlockSet to BlockDownloader logging
ascjones Sep 13, 2018
6612f96
Reset OldBlocks download from last enqueued
ascjones Sep 14, 2018
e72ad14
Ignore expired Body and Receipt requests
ascjones Sep 14, 2018
3d34ed5
Log when ancient block download being restarted
ascjones Sep 14, 2018
3fe9c92
Only request old blocks from peers with >= difficulty
ascjones Sep 17, 2018
757641d
Some logging and clear stalled blocks head
ascjones Sep 18, 2018
54614c8
Revert "Some logging and clear stalled blocks head"
ascjones Sep 19, 2018
39166d0
Reset stalled header if useless more than once
ascjones Sep 19, 2018
eea2f8d
Store useless headers in HashSet
ascjones Sep 20, 2018
d5d11c6
Merge branch 'master' into aj/fix-ancient-blocks-sync
ascjones Sep 20, 2018
535dc63
Add sync target to logging macro
ascjones Sep 20, 2018
68f26e8
Merge branch 'aj/fix-ancient-blocks-sync' into aj/ancient-blocks-rese…
ascjones Sep 20, 2018
9e18c90
Don't disable useless peer and fix log macro
ascjones Sep 20, 2018
409f3e8
Clear useless headers on reset and comments
ascjones Sep 20, 2018
df9f952
Use custom error for collecting blocks
ascjones Sep 21, 2018
10fc545
Remove blank line
ascjones Sep 21, 2018
8e7b813
Merge branch 'master' into aj/fix-ancient-blocks-sync
ascjones Sep 24, 2018
f0bb06d
Test for reset sync after consecutive useless headers
ascjones Sep 24, 2018
ec4adc0
Don't reset after consecutive headers when chain head
ascjones Sep 25, 2018
9b5f21f
Delete commented out imports
ascjones Sep 25, 2018
48e5483
Return DownloadAction from collect_blocks instead of error
ascjones Sep 26, 2018
a3409e3
Don't reset after round complete, was causing test hangs
ascjones Sep 26, 2018
4a03042
Add comment explaining reset after useless
ascjones Oct 1, 2018
4182c7a
Merge branch 'master' into aj/fix-ancient-blocks-sync
ascjones Oct 2, 2018
0af4cf6
Replace HashSet with counter for useless headers
ascjones Oct 2, 2018
b8418e4
Refactor sync reset on bad block/queue full
ascjones Oct 2, 2018
0ec672b
Add missing target for log message
ascjones Oct 2, 2018
3bfc2ae
Merge branch 'master' into aj/fix-ancient-blocks-sync
ascjones Oct 3, 2018
e72d150
Fix compiler errors and test after merge
ascjones Oct 3, 2018
d77ac44
ethcore: revert ethereum tests submodule update
andresilva Oct 9, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 19 additions & 5 deletions ethcore/sync/src/block_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,24 @@ impl BlockDownloader {
match self.state {
State::ChainHead => {
if !headers.is_empty() {
// TODO: validate heads better. E.g. check that there is enough distance between blocks.
trace!(target: "sync", "Received {} subchain heads, proceeding to download", headers.len());
self.blocks.reset_to(hashes);
self.state = State::Blocks;
return Ok(DownloadAction::Reset);
// When the round starts, subchain headers are requested
// with gaps to be filled. However if the round is reset
// while in State::Blocks then its possible to receive
// stale responses for the subchain heads in the gap. In
// this case the headers will have consecutive numbers
// so can be ignored here.
let is_subchain_heads = headers.len() == 1
|| headers.len() > 1
&& headers[1].header.number() - headers[0].header.number() > 1;

if is_subchain_heads {
trace!(target: "sync", "Received {} subchain heads, proceeding to download", headers.len());
self.blocks.reset_to(hashes);
self.state = State::Blocks;
return Ok(DownloadAction::Reset);
} else {
debug!(target: "sync", "Ignoring consecutive headers: expected subchain headers with gap");
}
} else {
let best = io.chain().chain_info().best_block_number;
let oldest_reorg = io.chain().pruning_info().earliest_state;
Expand Down Expand Up @@ -515,6 +528,7 @@ impl BlockDownloader {
},
Err(BlockImportError(BlockImportErrorKind::Queue(QueueErrorKind::Full(limit)), _)) => {
debug!(target: "sync", "Block import queue full ({}), restarting sync", limit);
bad = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is done to trigger resetting the sync right? Do you think we should return a different error type other than BlockDownloaderImportError::Invalid? (And also handle it in ChainSync::collect_blocks).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I agree and I had previously done that. However my idea with this initial PR was to do the minimum amount to fix this bug.

However looks like this fix doesn't work entirely so I will add that back while waiting for another mainnet sync...

break;
},
Err(e) => {
Expand Down