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

op-node: parallel events executor #4

Merged
merged 6 commits into from
Oct 28, 2024
Merged

Conversation

claymega
Copy link
Collaborator

@claymega claymega commented Oct 20, 2024

该分支源于 op-node: parallel events executor, 使处理event的handler能够并行执行 (event是derivation和sequence逻辑的步骤的抽象概念).

  • 添加了一个新的event executor: parallel deriver execution
  • 每个deriver有它自己的goroutine, 并行执行event;
  • event loop线程和其它handler线程的data race问题解决;

@claymega claymega changed the title Clay/feature/parallel events op-node: parallel events executor Oct 21, 2024
op-node/rollup/derive/pipeline.go Outdated Show resolved Hide resolved
op-node/rollup/derive/pipeline.go Outdated Show resolved Hide resolved
op-node/rollup/event/executor_parallel.go Outdated Show resolved Hide resolved
op-node/rollup/event/system.go Outdated Show resolved Hide resolved
op-node/rollup/sequencing/iface.go Outdated Show resolved Hide resolved
op-node/rollup/driver/steps.go Show resolved Hide resolved
Comment on lines 71 to 73
origin eth.L1BlockRef
resetL2Safe eth.L2BlockRef
resetSysConfig eth.SystemConfig
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we add mutex for this fields as well? Will they be modified concurrently by multiple goroutines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These fields are internal variables and will only be modified by the Step(), but this function will only be called in the pipeline goroutine and not be called by other handler goroutine.

Copy link
Collaborator

@Troublor Troublor Oct 23, 2024

Choose a reason for hiding this comment

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

@claymega I see Reset() function also modify this two variables and Reset() function may also be called by the main event loop. Can you confirm this will not cause a race?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@claymega I see Reset() function also modify this two variables and Reset() function may also be called by the main event loop. Can you confirm this will not cause a race?

Right, Reset() also modifies these two variables, and I re-check it, Reset() only be called in the pipeline goroutine

@claymega claymega requested a review from Troublor October 23, 2024 07:10
@claymega claymega merged commit 77c6b25 into develop Oct 28, 2024
3 checks passed
claymega added a commit that referenced this pull request Nov 1, 2024
* run op-node in parallel-event mode

* add metrics and fix data race

* delete debug log

* delete metrics

* recover executor_global_test.go

* modify the variable type and function name

* modify comment
claymega added a commit that referenced this pull request Nov 1, 2024
* run op-node in parallel-event mode

* add metrics and fix data race

* delete debug log

* delete metrics

* recover executor_global_test.go

* modify the variable type and function name

* modify comment
claymega added a commit that referenced this pull request Dec 17, 2024
* run op-node in parallel-event mode

* add metrics and fix data race

* delete debug log

* delete metrics

* recover executor_global_test.go

* modify the variable type and function name

* modify comment
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.

2 participants