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

[Execution] New ingestion engine #5593

Merged
merged 52 commits into from
May 1, 2024
Merged

Conversation

zhangchiqing
Copy link
Member

@zhangchiqing zhangchiqing commented Mar 27, 2024

Please review the following PRs first:

#5248
#5288
#5337

This PR adds a new ingestion engine (Ingestion.NewMachine) that is responsible for fetching data for unexecuted blocks.

The new ingestion engine could resolve the following issues:
#5161
#5217
#5298

The new ingestion engine can be turned on with a new flag --enable-new-ingestion-engine=true.

The new flag allows us to switch back to the old ingestion engine during the testing phase if the new engine ran into any problem, such as the block execution is halted.

It would not cause execution fork, because the ingestion engine is only responsible for determine when a block can be executed, rather than how a block is executed.

@zhangchiqing zhangchiqing changed the base branch from master to leo/ingestion-core-use-component-manager April 25, 2024 15:30
@zhangchiqing zhangchiqing marked this pull request as ready for review April 25, 2024 16:13
cmd/Dockerfile Outdated
@@ -37,7 +37,7 @@ WORKDIR /app

ARG GOARCH=amd64
# TAGS can be overriden to modify the go build tags (e.g. build without netgo)
ARG TAGS="netgo"
ARG TAGS="netgo,osusergo"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok with updating the default user, but I think this should go in a different PR since it has implications for node operators and infra

@@ -164,6 +164,8 @@ func (e *Core) launchWorkerToExecuteBlocks(ctx irrecoverable.SignalerContext, re
}

func (e *Core) OnBlock(header *flow.Header, qc *flow.QuorumCertificate) {
e.log.Debug().Msgf("received block %v (%v)", header.Height, qc.BlockID)
Copy link
Contributor

Choose a reason for hiding this comment

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

make the args fields?

@@ -120,6 +121,7 @@ func (exeConf *ExecutionConfig) SetupFlags(flags *pflag.FlagSet) {
flags.BoolVar(&exeConf.onflowOnlyLNs, "temp-onflow-only-lns", false, "do not use unless required. forces node to only request collections from onflow collection nodes")
flags.BoolVar(&exeConf.enableStorehouse, "enable-storehouse", false, "enable storehouse to store registers on disk, default is false")
flags.BoolVar(&exeConf.enableChecker, "enable-checker", true, "enable checker to check the correctness of the execution result, default is true")
flags.BoolVar(&exeConf.enableNewIngestionEngine, "enable-new-ingestion-engine", false, "enable new ingestion engine, default is false")
Copy link
Member Author

Choose a reason for hiding this comment

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

By default the new ingestion engine is disabled, and can be enabled by flag.

@@ -399,11 +399,14 @@ func prepareExecutionService(container testnet.ContainerConfig, i int, n int) Se
panic(err)
}

enableNewIngestionEngine := true
Copy link
Member Author

Choose a reason for hiding this comment

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

the localnet enables new ingestion engine by default

Base automatically changed from leo/ingestion-core-use-component-manager to master April 30, 2024 17:26
@zhangchiqing zhangchiqing disabled auto-merge May 1, 2024 16:45
@zhangchiqing zhangchiqing added this pull request to the merge queue May 1, 2024
Merged via the queue into master with commit e055e56 May 1, 2024
55 checks passed
@zhangchiqing zhangchiqing deleted the leo/ingestion-new-engine branch May 1, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants