Warpsync: epoch syncing #2367
Merged
Warpsync: epoch syncing #2367
Conversation
This file contains 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
Co-authored-by: Jonas Theis <[email protected]>
…himmer into refactor/p2p-gossip-split
karimodm
force-pushed
the
feat/epoch-syncing
branch
from
0381a85
to
a1ea58f
Compare
karimodm
force-pushed
the
feat/epoch-syncing
branch
from
dc5b11b
to
1eee490
Compare
jonastheis
reviewed
Aug 16, 2022
packages/node/warpsync/syncing.go
Outdated
Comment on lines
136
to
138
| if err != nil { | ||
| return errors.Errorf("sync failed for range %d-%d", start, end) | ||
| } |
There was a problem hiding this comment.
Instead of assigning the error in the callback I'd suggest the following: hand over a ctx that you can cancel to the goroutines and make them write into a result channel that contains an error and the completed EI (or maybe we don't even need that and counting is enough). Then you can cancel all goroutines on demand if there's an error and you wait on the result channel instead of the epochProcessingChan
There was a problem hiding this comment.
Seems to be addressed now with the resultChan, cancellable errCtx and errorChan.
karimodm
force-pushed
the
feat/epoch-syncing
branch
from
1500811
to
c56f92e
Compare
jonastheis
approved these changes
Aug 23, 2022
plugins/warpsync/parameters.go
Outdated
| // BlockBatchSize defines the amount of blocks to send in a single epoch blocks response"` | ||
| BlockBatchSize int `default:"100" usage:"the amount of blocks to send in a single epoch blocks response"` | ||
| // SyncRangeTimeOut defines the time after which a sync range is considered as failed. | ||
| SyncRangeTimeOut time.Duration `default:"30s" usage:"the time after which a sync range is considered as failed"` |
There was a problem hiding this comment.
Isn't that a little bit low as a default value?
There was a problem hiding this comment.
Increased it to 5m
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.
Description of change
Issue #2380
Implementation of the
Warpsyncplugin that will allow to retrieve entire epoch's content for a specific epoch commitment chain: for faster syncing and for switching to a heavier EC chain.Type of change