Range sync stuck due to insufficient peers on data column subnets when triggered

[jimmygchen]

Issue

When a head chain sync starts, if there isn't any peers on data column subnets, it will not progress and gets stuck unless some external events triggers it to resume (e.g. new peer added).

This scenario is easily reproducible with PeerDAS, because we don't immediately know the peer's custody_group_count until we get a metadata response back from them.

The sequence of events I observed:

1. Connected to multiple peers with advanced sync state, so we start a new head chain sync (log New chain added to sync, New head chain started syncing)
2. And immediately, sync thinks there isn't any peers on its custody data column subnets because we haven't obtained their metadata yet and don't know their custody count, therefore no batch gets sent (logWaiting for peers to be available on custody column subnets)
3. Later, we obtained their metadata (log Obtained peer's metadata), but this doesn't re-trigger range sync, so we're stuck until finalized sync kicks in.

A few possible solutions (not mutually exclusive):

1. Handle this in range request decoupling properly (Improve range sync with PeerDAS #6258)
2. Compute peer_info.custody_subnets when peer is connected, and using the minimum custody requirement, as every peer must serve the minimum required column count - this way we're likely to have some peers in data column subnets even before obtaining metadata.
3. Update sync when we obtain peer metadata, and trigger resume

Additional Info

Range sync currently rely on this check syncing_chain.good_peers_on_sampling_subnets before requesting batches from peer.

1. lighthouse/beacon_node/network/src/sync/range_sync/chain.rs

   Line 1075 in 7d54a43

   if !self.good_peers_on_sampling_subnets(epoch, network) {

2. lighthouse/beacon_node/network/src/sync/range_sync/chain.rs

   Lines 444 to 448 in 7d54a43

   	} else if !self.good_peers_on_sampling_subnets(self.processing_target, network) {
   	// This is to handle the case where no batch was sent for the current processing
   	// target when there is no sampling peers available. This is a valid state and should not
   	// return an error.
   	return Ok(KeepChain);

3. lighthouse/beacon_node/network/src/sync/range_sync/chain.rs

   Lines 1175 to 1181 in 7d54a43

   	if !self.good_peers_on_sampling_subnets(self.to_be_downloaded, network) {
   	debug!(
   	self.log,
   	"Waiting for peers to be available on custody column subnets"
   	);
   	return None;
   	}

This is a workaround to avoid sending out excessive block requests because block and data column requests are currently coupled. In the where we request a batch, and there's no peers on the required column subnet, the blocks request will be sent but the data columns by range won't, and will fail with RpcRequestSendError::NoCustodyPeers. This will trigger retry and the node end up sending excessive blocks by range requests to peers without progressing. Longer term solution is to decouple the ByRange requests (#6258).

[dapplion]

What if we change the AddPeer event to

    AddPeer(PeerId, SyncInfo, CustodyColumns),

So the peer manager waits to know the peer's columns before emitting it. Then sync doesn't need to rely on the network globals to compute how is custodying what

[jimmygchen]

Currently AddPeer is triggered on a peer status response, and then he event handler update the peer's sync status & sync state. If we wait for metadata response, it delays these from happening - do you see any issues with this?

Or Is it worth adding another event for metadata response, and just re-trigger column requests the same way you're triggering by root requests here:

lighthouse/beacon_node/network/src/sync/manager.rs

Lines 437 to 440 in 2193f6a

	// Try to make progress on custody requests that are waiting for peers
	for (id, result) in self.network.continue_custody_by_root_requests() {
	self.on_custody_by_root_result(id, result);
	}

Can we also start with the minimal custody requirement before we get metadata response back?

This seems to be the main issue with syncing a new node on devnet-4 right now, and would be good to fix ASAP.

[dapplion]

If we wait for metadata response, it delays these from happening - do you see any issues with this?

Hmm not really, RPC requests complete very fast and I don't see how this can be a bottleneck