Reduce fast-sync-min-peers for post merge #4298
Conversation
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
|
Does this overlap with #4285 |
|
@macfarla Reducing to two for all networks seems a bit dangerous imo. This may have to be done on a case-by-case basis depending on the networks. This PR focuses more on the post merge |
matkt
added
the
doc-change-required
|
@mark-terry can you review this one? |
| description = | ||
| "Minimum number of peers required before starting fast sync. (default: ${DEFAULT-VALUE})") | ||
| private final Integer fastSyncMinPeerCount = FAST_SYNC_MIN_PEER_COUNT; | ||
| description = "Minimum number of peers required before starting fast sync.") |
There was a problem hiding this comment.
is it possible to says which is the default pre and post merge?
since there is actually a default, it seems not correct to remove the info here
There was a problem hiding this comment.
I modified to add the default value again
| @@ -2783,10 +2783,19 @@ private Optional<PkiBlockCreationConfiguration> maybePkiBlockCreationConfigurati | |||
| } | |||
|
|
|||
| private SynchronizerConfiguration buildSyncConfig() { | |||
| Integer fastSyncMinPeers = fastSyncMinPeerCount; | |||
| if (fastSyncMinPeers == null) { | |||
| if (isMergedNetwork(network)) { | |||
There was a problem hiding this comment.
Is there more logic required here for the case where the sync begins at a pre-merge block? I've had cases recently on Ropsten where using fast sync with <=2 peers resulted in the sync stalling (ended up on a fork perhaps?)
There was a problem hiding this comment.
we cannot start a sync on a premerge block because of the snapsync. We need to use <128 old blocks. So after small amount of time it's not possible
There was a problem hiding this comment.
if the network merged the pivot block need to come from CL so 1 it's enough .
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
rolandtyler
removed
the
doc-change-required
…4298) Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
…4298) Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
…4298) Signed-off-by: Karim TAAM <karim.t2am@gmail.com> Signed-off-by: garyschulte <garyschulte@gmail.com>
…4298) Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Karim TAAM karim.t2am@gmail.com
PR description
Post merge we no longer need to wait to have 5 peers to validate a pivot block knowing that the pivot block comes from the consensus client. This PR proposes to reduce the min peers to 1 for post merge network. It will always be possible to change it via the flag if necessary
Fixed Issue(s)
fixes #4202
Documentation
doc-change-requiredlabel to this PR ifupdates are required.
Changelog