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

Tweaked snapshot params #6344

Merged
merged 1 commit into from Aug 22, 2017
Merged

Tweaked snapshot params #6344

merged 1 commit into from Aug 22, 2017

Conversation

arkpar
Copy link
Collaborator

@arkpar arkpar commented Aug 21, 2017

No description provided.

@arkpar arkpar added A0-pleasereview 🤓 Pull request needs code review. B0-patch M4-core ⛓ Core client code / Rust. labels Aug 21, 2017
@5chdn
Copy link
Contributor

5chdn commented Aug 21, 2017

CI timed out again.

Getting this, error message sounds super weird:

2017-08-21 13:53:06  Encountered error during block restoration: Snapshot contained too many blocks. Expected 5000, got 4960

Should be probably self.fed_blocks + num_blocks in the error message not self.fed_blocks.

https://github.com/paritytech/parity/blob/1892e1b62cc581972e00eee559ac8804e06de196/ethcore/src/snapshot/consensus/work.rs#L220-L222

Not sure if this is related, but never noticed this earlier. Once the snapshot restoration fails, the total amount of requested snapshots increases:

[...]
2017-08-21 13:53:04  Syncing snapshot 207/403        #0   37/256 peers   8 KiB chain 3 MiB db 0 bytes queue 19 KiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-08-21 13:53:06  Encountered error during block restoration: Snapshot contained too many blocks. Expected 5000, got 4960
2017-08-21 13:53:14  Syncing snapshot 2/609        #0   36/256 peers   8 KiB chain 3 MiB db 0 bytes queue 19 KiB sync  RPC:  0 conn,  0 req/s,   0 µs
[...]
2017-08-21 14:02:59  Syncing snapshot 273/609        #0   51/256 peers   8 KiB chain 3 MiB db 0 bytes queue 19 KiB sync  RPC:  0 conn,  0 req/s,   0 µs
2017-08-21 14:03:07  Encountered error during block restoration: Snapshot contained too many blocks. Expected 5000, got 4920
2017-08-21 14:03:09  Syncing snapshot 0/625        #0   51/256 peers   8 KiB chain 3 MiB db 0 bytes queue 19 KiB sync  RPC:  0 conn,  0 req/s,   0 µs
[...]

@arkpar arkpar force-pushed the block-limits branch 2 times, most recently from c5e0c2e to 677e2d0 Compare August 21, 2017 15:03
const SNAPSHOT_BLOCKS: u64 = 30000;
const SNAPSHOT_BLOCKS: u64 = 5000;
/// Maximum number of blocks allowed in an ethash snapshot.
const MAX_SNAPSHOT_BLOCKS: u64 = 300000;
Copy link
Contributor

Choose a reason for hiding this comment

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

is it meant to be 30k or 300k?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 22, 2017
@debris
Copy link
Collaborator

debris commented Aug 22, 2017

closes #6297

@arkpar arkpar merged commit d41dd13 into master Aug 22, 2017
@arkpar arkpar deleted the block-limits branch August 22, 2017 09:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants