-
Notifications
You must be signed in to change notification settings - Fork 180
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
Conversation
…stion-new-engine-not-work
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" |
There was a problem hiding this comment.
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
engine/execution/ingestion/core.go
Outdated
@@ -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) |
There was a problem hiding this comment.
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") |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
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.