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

Stuck lookup v6 #6658

Open
wants to merge 2 commits into
base: release-v6.0.1
Choose a base branch
from
Open

Conversation

dapplion
Copy link
Collaborator

@dapplion dapplion commented Dec 5, 2024

Issue Addressed

PR

Introduced a regression on the lookup pruning logic. If a lookup loses all peers before the block request downloads the block, it will get stuck. This is a relatively frequent occurrence. This bug will result in occasional warn logs on the user's terminals.

Example case events

  • Lookup created for block A, from peer X
  • Peer X disconencts, lookup A now has 0 peers
  • 10 seconds pass
  • drop_lookups_without_peers runs on lookup A
    • has_no_peers returns true
    • elapsed condition returns true
    • is_awaiting_event returns true <- so it doesn't get pruned

is_awaiting_event checks:

  • awaiting_parent.is_some() returns false
  • self.block_request_state.state.is_awaiting_event() returns false (block awaiting download)
  • component_requests is WaitingForBlock, so it returns true <- the bug is here

// If components are waiting for the block request to complete, here we should
// check if the`block_request_state.state.is_awaiting_event(). However we already
// checked that above, so `WaitingForBlock => false` is equivalent.
ComponentRequests::WaitingForBlock => false,

Proposed Changes

Fix is_awaiting_event logic.

@michaelsproul michaelsproul added the v6.0.1 Bugfix for v6.0.0 label Dec 5, 2024
michaelsproul added a commit that referenced this pull request Dec 5, 2024
Squashed commit of the following:

commit 193c7f8
Author: dapplion <[email protected]>
Date:   Thu Dec 5 09:32:57 2024 +0800

    Fix stuck lookups if no peers on v6
michaelsproul added a commit that referenced this pull request Dec 9, 2024
Squashed commit of the following:

commit 193c7f8
Author: dapplion <[email protected]>
Date:   Thu Dec 5 09:32:57 2024 +0800

    Fix stuck lookups if no peers on v6
michaelsproul added a commit that referenced this pull request Dec 13, 2024
Squashed commit of the following:

commit 9d82a1d
Merge: 193c7f8 4946343
Author: Michael Sproul <[email protected]>
Date:   Fri Dec 13 11:48:03 2024 +1100

    Merge branch 'release-v6.0.1' into stuck-lookup-v6

commit 193c7f8
Author: dapplion <[email protected]>
Date:   Thu Dec 5 09:32:57 2024 +0800

    Fix stuck lookups if no peers on v6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v6.0.1 Bugfix for v6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants