Skip to content

Add reason of NoRequestNeeded to lookup sync#6317

Merged
mergify[bot] merged 2 commits into
sigp:unstablefrom
dapplion:peerdas-reason
Aug 30, 2024
Merged

Add reason of NoRequestNeeded to lookup sync#6317
mergify[bot] merged 2 commits into
sigp:unstablefrom
dapplion:peerdas-reason

Conversation

@dapplion

@dapplion dapplion commented Aug 28, 2024

Copy link
Copy Markdown
Collaborator

Issue Addressed

Porting improvement from

Proposed Changes

Now there are more reasons why a lookup request will be NoRequestNeeded. To debug stuck sync lookup it's important to know why sync would consider the request unnecessary.

@dapplion dapplion added ready-for-review The code is ready for review das Data Availability Sampling labels Aug 28, 2024
@dapplion dapplion mentioned this pull request Aug 28, 2024
Closed
jimmygchen
jimmygchen approved these changes Aug 30, 2024
View reviewed changes

@jimmygchen jimmygchen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, I think this will help with debugging 👍

@jimmygchen jimmygchen added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Aug 30, 2024
@jimmygchen

jimmygchen commented Aug 30, 2024

Copy link
Copy Markdown
Member

@mergify queue

@mergify

mergify Bot commented Aug 30, 2024 •
edited
Loading

Copy link
Copy Markdown

queue

🛑 The pull request has been removed from the queue default

Details

The pull request conflicts with at least one pull request ahead in queue.

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.

@jimmygchen
Merge branch 'unstable' into peerdas-reason
bef51ed 
# Conflicts:
#	beacon_node/network/src/sync/network_context.rs
@jimmygchen

jimmygchen commented Aug 30, 2024

Copy link
Copy Markdown
Member

@mergify queue

@mergify

mergify Bot commented Aug 30, 2024 •
edited
Loading

Copy link
Copy Markdown

queue

🛑 The pull request has been removed from the queue default

Details

The merge 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.

mergify Bot added a commit that referenced this pull request Aug 30, 2024
@mergify
Merge of #6317
b90797d
@mergify mergify Bot mentioned this pull request Aug 30, 2024
Closed
5 tasks
@jimmygchen

jimmygchen commented Aug 30, 2024

Copy link
Copy Markdown
Member

@mergify requeue

@mergify

mergify Bot commented Aug 30, 2024

Copy link
Copy Markdown

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

mergify Bot commented Aug 30, 2024 •
edited
Loading

Copy link
Copy Markdown

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at 60157ad

mergify Bot added a commit that referenced this pull request Aug 30, 2024
@mergify
Merge of #6317
0d0dc8b
@mergify mergify Bot mentioned this pull request Aug 30, 2024
Closed
5 tasks
@mergify mergify Bot merged commit 60157ad into sigp:unstable Aug 30, 2024
@dapplion dapplion deleted the peerdas-reason branch September 2, 2024 12:57
AgeManning pushed a commit to AgeManning/lighthouse that referenced this pull request Sep 3, 2024
@dapplion @AgeManning
Add reason of NoRequestNeeded to lookup sync (sigp#6317)
1f5f0e4 
* Add reason of NoRequestNeeded to lookup sync

* Merge branch 'unstable' into peerdas-reason

# Conflicts:
#	beacon_node/network/src/sync/network_context.rs
chong-he pushed a commit to chong-he/lighthouse that referenced this pull request Nov 26, 2024
@dapplion @chong-he
Add reason of NoRequestNeeded to lookup sync (sigp#6317)
f51703e 
* Add reason of NoRequestNeeded to lookup sync

* Merge branch 'unstable' into peerdas-reason

# Conflicts:
#	beacon_node/network/src/sync/network_context.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jimmygchen jimmygchen jimmygchen approved these changes

@realbigsean realbigsean Awaiting requested review from realbigsean

Assignees

No one assigned

Labels

das Data Availability Sampling ready-for-merge This PR is ready to merge.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dapplion @jimmygchen