Skip to content

Remove reprocess channel #7437

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

Merged
merged 13 commits into from
Jun 20, 2025
Merged

Conversation

eserilev
Copy link
Member

@eserilev eserilev commented May 11, 2025

Issue Addressed

Partially #6291

Proposed Changes

This PR removes the reprocess event channel from being externally exposed. All work events are now sent through the single BeaconProcessorSend channel. I've introduced a new Work::Reprocess enum variant which we then use to schedule jobs for reprocess. I've also created a new scheduler module which will eventually house the different scheduler impls.

This is all needed as an initial step to generalize the beacon processor

A "full" implementation for the generalized beacon processor can be found here
#6448

I'm going to try to break up the full implementation into smaller PR's so it can actually be reviewed

@eserilev eserilev requested a review from jxs as a code owner May 11, 2025 06:30
@michaelsproul
Copy link
Member

Nice! Love this!

@eserilev
Copy link
Member Author

eserilev commented May 12, 2025

Since reprocess and regular work events will all come through a single channel, we may need to raise the default work queue size like we're doing here: #7415

@eserilev eserilev added the ready-for-review The code is ready for review label May 12, 2025
Copy link

mergify bot commented May 16, 2025

This pull request has merge conflicts. Could you please resolve them @eserilev? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels May 16, 2025
Copy link

mergify bot commented May 17, 2025

Some required checks have failed. Could you please take a look @eserilev? 🙏

Copy link

mergify bot commented May 19, 2025

All required checks have passed and there are no merge conflicts. This pull request may now be ready for another review.

@mergify mergify bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 19, 2025
@michaelsproul michaelsproul added the v7.1.0 Post-Electra release label Jun 16, 2025
Copy link

mergify bot commented Jun 16, 2025

This pull request has merge conflicts. Could you please resolve them @eserilev? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 16, 2025
@michaelsproul michaelsproul mentioned this pull request Jun 16, 2025
@mergify mergify bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 17, 2025
Copy link

mergify bot commented Jun 17, 2025

This pull request has merge conflicts. Could you please resolve them @eserilev? 🙏

@mergify mergify bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels Jun 17, 2025
@mergify mergify bot added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jun 17, 2025
eserilev added a commit that referenced this pull request Jun 17, 2025
Squashed commit of the following:

commit 3732caf11917ebe0c98c1ac0f1b3598fdf151988
Merge: 1b1aee5 9b150f0
Author: Eitan Seri-Levi <[email protected]>
Date:   Tue Jun 17 21:24:46 2025 +0300

    resolve merge conflict

commit 1b1aee5
Merge: 95b780c 6786b9d
Author: Eitan Seri-Levi <[email protected]>
Date:   Tue Jun 17 18:46:05 2025 +0300

    resolve merge conflicts

commit 95b780c
Merge: 489abdf 6135f41
Author: Michael Sproul <[email protected]>
Date:   Mon Jun 16 17:21:04 2025 +1000

    Merge remote-tracking branch 'origin/unstable' into remove-reprocess-channel

commit 489abdf
Merge: e87d44b 50dbfdf
Author: Eitan Seri-Levi <[email protected]>
Date:   Mon May 19 13:24:12 2025 -0700

    Merge branch 'unstable' into remove-reprocess-channel

commit e87d44b
Merge: c3406eb 23ad833
Author: Eitan Seri-Levi <[email protected]>
Date:   Sat May 17 10:40:31 2025 -0700

    merge conflicts

commit c3406eb
Merge: 0ddb782 c0f220b
Author: Eitan Seri-Levi <[email protected]>
Date:   Sun May 11 18:20:54 2025 -0700

    Merge branch 'remove-reprocess-channel' of https://github.com/eserilev/lighthouse into remove-reprocess-channel

commit 0ddb782
Author: Eitan Seri-Levi <[email protected]>
Date:   Sun May 11 18:20:38 2025 -0700

    add scheduler mod

commit c0f220b
Merge: e1ae209 5933901
Author: Eitan Seri-Levi <[email protected]>
Date:   Sun May 11 17:40:14 2025 -0700

    Merge branch 'unstable' into remove-reprocess-channel

commit e1ae209
Author: Eitan Seri-Levi <[email protected]>
Date:   Sun May 11 00:20:53 2025 -0700

    fix test

commit 110c4cc
Author: Eitan Seri-Levi <[email protected]>
Date:   Sat May 10 23:55:53 2025 -0700

    fix

commit 39c5411
Author: Eitan Seri-Levi <[email protected]>
Date:   Sat May 10 23:46:03 2025 -0700

    add allow_reprocess flag

commit a8f3afe
Author: Eitan Seri-Levi <[email protected]>
Date:   Sat May 10 23:38:13 2025 -0700

    fix tests

commit 3c63ff3
Author: Eitan Seri-Levi <[email protected]>
Date:   Sat May 10 23:24:33 2025 -0700

    Remove reprocess channel
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

LGTM, and seems to be running well on our infra!

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Jun 20, 2025
@mergify mergify bot merged commit f67084a into sigp:unstable Jun 20, 2025
33 checks passed
mergify bot pushed a commit that referenced this pull request Jul 10, 2025
Post-Pectra release for tree-states hot 🎉

Already merged to `release-v7.1.0`:

- #7444
- #6750
- #7437
- #7133
- #7620
- #7663
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-quality ready-for-merge This PR is ready to merge. v7.1.0 Post-Electra release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants