Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add blockhashes stage #862

Merged
merged 20 commits into from
Aug 10, 2020
Merged

add blockhashes stage #862

merged 20 commits into from
Aug 10, 2020

Conversation

Giulio2002
Copy link
Collaborator

No description provided.

@AskAlexSharov
Copy link
Collaborator

@Giulio2002 plz add new stage to list of disabled stages in cmd/integration/commands/state_stages.go
because ./cmd/integration state_stages assume that DB already has Headers,Bodies,Senders ready.

@AlexeyAkhunov AlexeyAkhunov changed the title add blockhashes stage [WIP] add blockhashes stage Aug 5, 2020
@AskAlexSharov
Copy link
Collaborator

I think this ticket needs migration - to cover next case:

  • TG was interrupted at stage6
  • TG upgraded
  • TG started on version with this feature included - it starts from stage6, not from 0
  • If some stage >=6 will use bucket created by this stage - bucket will empty

FYI: TG starts from interrupted stage, not from 0 - by only reason - stage5 expecting that stage6 executed. If stages execute in next order: 5, 6 interrupted, 0, 1, 2, 3, 4, 5 fail - expected 6 executed on previous iteration.

)

func extractHeaders(k []byte, v []byte, next etl.ExtractNextFunc) error {
if len(k) != 40 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Put a comment here why you are comparing with 40

); err != nil {
return err
}
s.Done()
Copy link
Contributor

Choose a reason for hiding this comment

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

this should probably be DoneAndUpdate(headNumber) to make sure we save the progress

}

func SpawnBlockHashStage(s *StageState, stateDB ethdb.Database, quit <-chan struct{}) error {
firstHash := common.BytesToHash(s.StageData)
Copy link
Contributor

Choose a reason for hiding this comment

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

We stopped actively using StageData and OnLoadCommit, because it creates some complications. It is easier just to start the whole operation over. As long as it is idempotent (applying it more than once has the same effect as applying once has the same effect), it is a more reliable way to deal with. This means that if you interrupt this stage and restart, it will start from the beginning. I remember you were asking me about it, but I forgot the context, sorry

@AlexeyAkhunov
Copy link
Contributor

Is there any way to add a couple of unit tests for this?

@AlexeyAkhunov
Copy link
Contributor

There also needs to be a migration implemented to fill in the progress of the new BlockHashes stage for the people who did not have it, but have already synced without it (and have all the correct entries in the database)

go.mod Outdated
@@ -23,6 +23,7 @@ require (
github.com/dop251/goja v0.0.0-20200219165308-d1232e640a87
github.com/edsrzf/mmap-go v0.0.0-20160512033002-935e0e8a636c
github.com/ethereum/evmc/v7 v7.3.0
github.com/ethereum/go-ethereum v1.9.18
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a dependency?

}

var unwindStagedsyncToUseStageBlockhashes = Migration{
Name: "stagedsync_to_use_stage_blockhashes",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename this so it does not use the same name as the other one

@AlexeyAkhunov AlexeyAkhunov changed the title [WIP] add blockhashes stage add blockhashes stage Aug 10, 2020
@AlexeyAkhunov AlexeyAkhunov merged commit 0578949 into master Aug 10, 2020
@vorot93 vorot93 deleted the stage_blockhash branch February 4, 2021 09:02
battlmonstr pushed a commit that referenced this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants