Expose config max-round-blocks-to-import #9439
Conversation
sorpaas
added
A0-pleasereview 🤓
5chdn
added
the
B7-releasenotes 📜
| @@ -98,6 +98,7 @@ pub struct ImportBlockchain { | |||
| pub with_color: bool, | |||
| pub verifier_settings: VerifierSettings, | |||
| pub light: bool, | |||
| pub max_round_blocks_to_import: usize, | |||
There was a problem hiding this comment.
We probably don't need to add this field here and just use the default (12). Same below for Export* structs.
There was a problem hiding this comment.
--max-round-blocks-to-import is a valid args for ImportBlockchain and Snapshot commands anyway, so I think it would be better to include them there, just in case it caught people in surprise? For example, --max-round-blocks-to-import do have (small) impact because the execution would call import_verified_blocks.
There was a problem hiding this comment.
It is a valid option indeed, I just assumed it wouldn't make much of a difference without networking. I'm OK with keeping it configurable.
| @@ -62,6 +62,7 @@ pub struct SnapshotCommand { | |||
| pub file_path: Option<String>, | |||
| pub kind: Kind, | |||
| pub block_at: BlockId, | |||
| pub max_round_blocks_to_import: usize, | |||
There was a problem hiding this comment.
Same as above, I think we only need to add this for the RunCmd.
5chdn
added
A6-mustntgrumble 💦
|
Did you experience any performance improvements/decreases defaulting from 4 to 12? |
|
@ngotchac No. Not sure how to benchmark this. |
|
Nothing really precise here, but just looking empirically how fast can you sync to block #N I guess? I think it won't change much though. |
|
Are you still working on this? |
5chdn
added
the
A3-stale 🍃
…-max-round-blocks-to-import
|
@5chdn Fixed! Didn't realize there were merge conflicts! |
sorpaas
added
A0-pleasereview 🤓
| @@ -785,6 +785,10 @@ usage! { | |||
| "--stratum-secret=[STRING]", | |||
| "Secret for authorizing Stratum server for peers.", | |||
|
|
|||
| ARG arg_max_round_blocks_to_import: (usize) = 12usize, or |c: &Config| c.mining.as_ref()?.max_round_blocks_to_import.clone(), | |||
There was a problem hiding this comment.
Doesn't it fit more into Performance section?
|
please rebase on master 🙈 |
…-max-round-blocks-to-import
Fixes #9320.
parity-ethereum imports blocks in rounds. If at the end of any round, the queue is not empty, we consider it to be "importing" and won't notify pubsub. On large re-orgs (10+ blocks), this is possible.
max_round_blocks_to_importnumber from4to12.--max-round-blocks-to-import [S]. With unstable network conditions, it is advised to increase the number. This shouldn't have any noticeable performance impact unless the number is set to really large.