net/peerset: Optimize substream opening duration for SetReservedPeers

[lexnv]

While triaging the Versi-net, I've discovered that the connection between collators and validators sometimes takes less than 20ms, while at other times it takes more than 500ms.

In both cases, the validators are already connected to a different protocol. Therefore, opening and negotiating substreams must be almost instant.

The slot timer of the peerset artificially introduces the delay:

• The SetReservedPeers is received by the peerset. At this step, the peerset propagated the closedSubstream to signal that it wants to disconnect previously reserved peers.
• At the next slot allocation timer tick (after 1s), the newly added reserved peers are requested to be connected

This can introduce an artificial delay of up to 1s, which is unnecessary.

To mitigate this behavior, this PR:

• Transforms the enum PeersetNotificationCommand into a structure. Effectively, the peerset can specify directly to close some substreams and open other substreams
• Upon receiving the SetReservedPeers command, peers are moved into the Opening state and the request is propagated to the litep2p to open substreams.
• The behavior of the slot allocation timer remains identical. This is needed to capture the following edge cases:
  • The reserved peer of the SetReservedPeers is not disconnected, but backoff / pending closing.
  • The reserved peer is banned

cc @paritytech/networking

Detected during versi-net triaging of elastic scaling: #10310 (comment)

[lexnv]

/cmd prdoc --audience node_dev --bump patch

[bkchr]

Suggested change

	log::debug!(target: LOG_TARGET, "{}: open substreams to {:?}", self.protocol, command.open_peers);
	let _ = self
	.handle
	.open_substream_batch(command.open_peers.into_iter().map(From::from))
	.await;
	}
	self.handle.close_substream_batch(peers.into_iter().map(From::from)).await;
	},
	if !command.close_peers.is_empty() {
	log::debug!(target: LOG_TARGET, "{}: close substreams to {:?}", self.protocol, command.close_peers);
	self.handle
	log::trace!(target: LOG_TARGET, "{}: open substreams to {:?}", self.protocol, command.open_peers);
	let _ = self
	.handle
	.open_substream_batch(command.open_peers.into_iter().map(From::from))
	.await;
	}
	if !command.close_peers.is_empty() {
	log::trace!(target: LOG_TARGET, "{}: close substreams to {:?}", self.protocol, command.close_peers);
	self.handle

[lexnv]

Makes sense, have downgraded the logs to trace since git doesn't want to apply the patch 🙏

[dmitry-markin]

nit: connect_reserved_peers goes over all peers (not only reserved) to find disconnected reserved peers. We can handle only newly added reserved peers here like it's done with reserved_peers_maybe_remove, and let old reserved peers that got disconnected to be handled later on slot allocation. This should optimize things for the case when the list of reserved peers is updated many times per second.

This is minor though, feel free to ignore.

[lexnv]

Totally makes sense, I've copy pasted the code without thinking too much about it:

• now we are only iterating over the provided reserved peers
• in a single iteration I'm also changing the state to opening, since there's no real reason why we'd have to pay for a second pass
• slightly adjusted the suggestion and picked iterating through all reserved peers since they aren't expected to be large, and wanted to "connect" as fast as possible

[paritytech-release-backport-bot]

Successfully created backport PR for stable2509:

• [stable2509] Backport #10362: net/peerset: Optimize substream opening duration for SetReservedPeers #10408

[paritytech-release-backport-bot]

Successfully created backport PR for stable2512:

• [stable2512] Backport #10362: net/peerset: Optimize substream opening duration for SetReservedPeers #10409