Skip to content

op-node: Switch L1 Traversal to a pull based model#3583

Merged
mslipper merged 10 commits intodevelopfrom
jg/l1traversal
Sep 28, 2022
Merged

op-node: Switch L1 Traversal to a pull based model#3583
mslipper merged 10 commits intodevelopfrom
jg/l1traversal

Conversation

@trianglesphere
Copy link
Contributor

@trianglesphere trianglesphere commented Sep 27, 2022

Description

The L1 Retrieval stage is now responsible for pulling data from the L1 Traversal stage. In addition, the pipeline is responsible for advancing the state of the L1 Traversal stage.

The L1 Traversal stage only provides access to the current L1 block once - it pretends to be a queue that is consumed from.

Tests

I had to modify some of the unit tests, but they are passing.

Metadata

  • Fixes ENG-2747

@changeset-bot
Copy link

changeset-bot bot commented Sep 27, 2022

⚠️ No Changeset found

Latest commit: 6890e77

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mergify
Copy link
Contributor

mergify bot commented Sep 27, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

@trianglesphere trianglesphere marked this pull request as ready for review September 28, 2022 18:40
@trianglesphere
Copy link
Contributor Author

trianglesphere commented Sep 28, 2022

The hive p2p test seems to be failing on this PR, but it is fixed in the next PR + is working on the final PR.

Edit: fixed now

This makes the L1 Retrieval a purely pull based stage. This commit
required large modifications to the channel bank stage in order
for the channel bank to maintain it's own progress.
The L1 Retrieval stage is now responsible for pulling data from
the L1 Traversal stage. In addition, the pipeline is responsible
for advancing the state of the L1 Traversal stage.

The L1 Traversal stage only provides access to the current L1 block
once - it pretends to be a queue that is consumed from.
Copy link
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

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

LGTM, nice work. After all stage refactor PRs land we can revisit the pipeline function itself to get rid of the separate types of resets (old resets and new pull based resets).

trianglesphere and others added 7 commits September 28, 2022 15:23
This again requires a fair amount of changes to channel_in_reader.go
for the channel in reader to maintain its progress state.
Like the rest of the changes, this also required modifications to
the next stage - the batch queue in order for it to manage the
progress API. This commit required even more changes than usual.
I changed the pipeline to be reset to a common starting point
and now use the L2SafeHead block to filter out adding batches &
L1 blocks to the batch queue.
The attributes queue actually had pretty few modifications to work
with the progress API. The logic of switching the batch queue over
was a bit more complex because the batch queue is very stateful, but
still not the worst.
The progress API is very nearly removed from the engine queue stage.
op-node: Switch batch queue to be pull based
op-node: Switch channel bank and channel in reader to a pull based stage
op-node: Switch L1 Retrieval to pull based
@mslipper mslipper merged commit 6a5fbf1 into develop Sep 28, 2022
@mslipper mslipper deleted the jg/l1traversal branch September 28, 2022 23:08
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.

3 participants