Attempt to continue lookups after adding peers

[dapplion]

Issue Addressed

Testing v5.2.1 a node hit a corner case of lookup sync causing a lookup to get stuck:

The lookup was originally created from an unknown parent block event with a block as child components. Therefore the original state of each request was:

• block: AwaitingProcess
• blobs: AwaitingDownload

When the parent imported successfully, it got called lookup continue_request. For each request:

• block: request is in AwaitingProcess so the block was sent to processing
• blobs: request is AwaitingDownload but the lookup has 0 peers so it doesn't make progress

lighthouse/beacon_node/network/src/sync/block_lookups/single_block_lookup.rs

Line 202 in f1d88ba

return Ok(());

When the block completes processing and it gets the MissingComponents, continue_request is called again, but the lookup still has 0 peers. So no progress is made.

Then immediately after that attempt at continue_request the lookup gets peers. When the drop_lookups_without_peers function runs, the lookup has peers but nothing is trying to make progress on it.

It appears the comment below is too optmistic. Yes this situation is a rare case but it results in lookups getting stuck. We should attempt to make progress on lookups when adding peers

lighthouse/beacon_node/network/src/sync/block_lookups/mod.rs

Lines 852 to 856 in f1d88ba

	// We may choose to attempt to continue a lookup here. It is possible that a lookup had zero
	// peers and after adding this set of peers it can make progress again. Note that this
	// recursive function iterates from child to parent, so continuing the child first is weird.
	// However, we choose to not attempt to continue the lookup for simplicity. It's not
	// strictly required and just and optimization for a rare corner case.

Proposed Changes

Attempt to make progress on existing lookups that are not awaiting parent and got new peers

[realbigsean]

LGTM

[pawanjay176]

Nice

[jimmygchen]

Looks good to me too!
It's indeed a rare case (wasn't easy for me to reproduce) and seem to happen on only few overwhelmed node mostly.

[jimmygchen]

I'm merging this into the testing branch. Please do not merge to release branch yet.

[jimmygchen]

@mergify queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at b38019c