Bor waypoint storage #9793
Bor waypoint storage #9793
Conversation
mh0lt
requested review from
shohamc1,
battlmonstr,
Giulio2002,
AskAlexSharov and
taratorio
| }, | ||
| indexes: []Index{Indexes.TxnHash, Indexes.TxnHash2BlockNum}, | ||
| indexBuilder: IndexBuilderFunc( | ||
| func(ctx context.Context, sn FileInfo, chainConfig *chain.Config, tmpDir string, p *background.Progress, lvl log.Lvl, logger log.Logger) (err error) { |
There was a problem hiding this comment.
I prefer this to be a named function.
| defer d.EnableMadvNormal().DisableReadAhead() | ||
| defer bodiesSegment.EnableMadvNormal().DisableReadAhead() | ||
|
|
||
| RETRY: |
There was a problem hiding this comment.
consider using a for loop
| @@ -117,6 +116,207 @@ func fetchAndWriteHeimdallSpan( | |||
| return spanID, nil | |||
| } | |||
|
|
|||
| func fetchRequiredHeimdallCheckpointsIfNeeded( | |||
There was a problem hiding this comment.
revise the function name:
- not clear what is meant by "required"?
- there's a function fetchAndWrite, so this one is not expected to write, or it should it be fetchAndWrite as well?
alternative names: fetchAndWriteMissingHeimdallCheckpoints, fetchAndWriteHeimdallCheckpointsIfNeeded
There was a problem hiding this comment.
I have just followed the existing language used in this file. I guess I can change all of them to make them more consistent.
polygon/bor/snaptype/types.go
Outdated
| }), | ||
| []snaptype.Index{snaptype.Indexes.BorTxnHash}, | ||
| snaptype.IndexBuilderFunc( | ||
| func(ctx context.Context, sn snaptype.FileInfo, chainConfig *chain.Config, tmpDir string, p *background.Progress, lvl log.Lvl, logger log.Logger) (err error) { |
There was a problem hiding this comment.
I prefer this to be a named function.
| @@ -72,15 +78,20 @@ func (m *Milestone) MarshalJSON() ([]byte, error) { | |||
| } | |||
|
|
|||
| func (m *Milestone) UnmarshalJSON(b []byte) error { | |||
|
|
|||
| // TODO - do we want to handle milestone_id ? | |||
There was a problem hiding this comment.
what do you mean? what benefits doest it have? what if we don't handle it?
There was a problem hiding this comment.
I don't know - hence the question. There seems to be a long hiemdall id sent on the json message. I don't know what it is, it seems different from the id passed between erigon & heimdall. I assume we can ignore it but am not 100% sure.
There was a problem hiding this comment.
I suggest to replace a TODO comment with a comment like: ignoring an unused milestone_id, which looks like ... to not bother us with thinking about whether or not to do this TODO :) I feel that TODO is like a promise, and I prefer to follow a rule that all TODOs are either fixed, removed or moved to the task tracker within a limited time frame.
|
ghost
mentioned this pull request
Implementation of db and snapshot storage for additional synced hiemdall waypoint types
This is targeted at the Astrid downloader which uses waypoints to verify headers during syncing and fork choice selection.
Post milestones for heimdall these types are currently downloaded by erigon but not persisted locally. This change adds persistence for these types.
In addition to the pure persistence changes this PR also contains a refactor step which is part of the process of extracting polygon related types from erigon core into a seperate package which may eventually be extracted to a separate module and possibly repo.
The aim is rather than the core
turbo\snapshotsync\freezeblockshaving to know about types it manages and how to exaract and index their contents this can concern it self with a set of macro shard management actions.This process is partially completed by this PR, a final step will be to remove BorSnapshots and to simplify the places in the code which has to remeber to deal with them. This requires further testing so has been left out of this PR to avoid delays in delivering the base types.
Status
Testing
This has been tested as follows:
Post merge steps