Skip to content

Sync lookup dedup range and blobs#5561

Merged
mergify[bot] merged 6 commits intosigp:unstablefrom
dapplion:sync-lookup-dedup-range-and-blobs
Apr 12, 2024
Merged

Sync lookup dedup range and blobs#5561
mergify[bot] merged 6 commits intosigp:unstablefrom
dapplion:sync-lookup-dedup-range-and-blobs

Conversation

@dapplion
Copy link
Collaborator

@dapplion dapplion commented Apr 11, 2024

Issue Addressed

Part of

Proposed Changes

Merges BackFillBlocks, BackFillBlockAndBlobs, RangeBlocks, RangeBlockAndBlobs handling. All those requests return the same type of data, a Vec, which can be handled with an enum request id to differentiate RangeSync from BackfillSync.

Pre-deneb requests of only blocks and post-deneb requests of blocks and blobs can be handled together by tracking if a blobs request has been sent.

This PR does not change any logic or behaviour of either sync, nor their tests. @realbigsean please look into this line change https://github.com/sigp/lighthouse/pull/5561/files#diff-3d6c8081703730f63787240a11f38fdde1d721397245cf316207b108f1df3defR962 to ensure it does not regress this fix #4869

Copy link
Member

@realbigsean realbigsean left a comment

Choose a reason for hiding this comment

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

@realbigsean please look into this line change https://github.com/sigp/lighthouse/pull/5561/files#diff-3d6c8081703730f63787240a11f38fdde1d721397245cf316207b108f1df3defR962 to ensure it does not regress this fix #4869

looks like the logic there is unchanged 👌

@realbigsean realbigsean added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Apr 11, 2024
@dapplion dapplion requested a review from realbigsean April 12, 2024 02:02
@dapplion dapplion 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 Apr 12, 2024
@realbigsean
Copy link
Member

@Mergifyio queue

@realbigsean realbigsean added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Apr 12, 2024
@mergify
Copy link

mergify bot commented Apr 12, 2024

queue

🛑 The pull request has been removed from the queue default

Details

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@realbigsean
Copy link
Member

@Mergifyio requeue

@mergify
Copy link

mergify bot commented Apr 12, 2024

requeue

✅ This pull request will be re-embarked automatically

Details

The followup queue command will be automatically executed to re-embark the pull request

@mergify
Copy link

mergify bot commented Apr 12, 2024

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at 6fb0b2e

mergify bot added a commit that referenced this pull request Apr 12, 2024
@mergify mergify bot merged commit 6fb0b2e into sigp:unstable Apr 12, 2024
@dapplion dapplion deleted the sync-lookup-dedup-range-and-blobs branch April 13, 2024 00:17
@dapplion dapplion mentioned this pull request Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge This PR is ready to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants