From c36fdaa58edd3db95dd1bdd40a1725db5a6637d5 Mon Sep 17 00:00:00 2001 From: "paritytech-cmd-bot-polkadot-sdk[bot]" <179002856+paritytech-cmd-bot-polkadot-sdk[bot]@users.noreply.github.com> Date: Fri, 17 Jan 2025 12:46:13 +0100 Subject: [PATCH] backport/stable2412: litep2p: Provide partial results to speedup GetRecord queries (#7192) Backport #7099 into `stable2412` from lexnv. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. --------- Signed-off-by: Alexandru Vasile Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Co-authored-by: Alexandru Vasile Co-authored-by: Egor_P --- Cargo.lock | 4 +- Cargo.toml | 2 +- prdoc/pr_7099.prdoc | 16 +++ .../client/network/src/litep2p/discovery.rs | 33 ++++-- substrate/client/network/src/litep2p/mod.rs | 100 ++++++++---------- 5 files changed, 87 insertions(+), 68 deletions(-) create mode 100644 prdoc/pr_7099.prdoc diff --git a/Cargo.lock b/Cargo.lock index 9b8019c205c8..af5c3b630931 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9311,9 +9311,9 @@ dependencies = [ [[package]] name = "litep2p" -version = "0.8.4" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2b0fef34af8847e816003bf7fdeac5ea50b9a7a88441ac927a6166b5e812ab79" +checksum = "6ca6ee50a125dc4fc4e9a3ae3640010796d1d07bc517a0ac715fdf0b24a0b6ac" dependencies = [ "async-trait", "bs58", diff --git a/Cargo.toml b/Cargo.toml index 354031a618e4..6b83eb7b4e47 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -846,7 +846,7 @@ linked-hash-map = { version = "0.5.4" } linked_hash_set = { version = "0.1.4" } linregress = { version = "0.5.1" } lite-json = { version = "0.2.0", default-features = false } -litep2p = { version = "0.8.4", features = ["websocket"] } +litep2p = { version = "0.9.0", features = ["websocket"] } log = { version = "0.4.22", default-features = false } macro_magic = { version = "0.5.1" } maplit = { version = "1.0.2" } diff --git a/prdoc/pr_7099.prdoc b/prdoc/pr_7099.prdoc new file mode 100644 index 000000000000..58d809f3c090 --- /dev/null +++ b/prdoc/pr_7099.prdoc @@ -0,0 +1,16 @@ +title: Provide partial results to speedup GetRecord queries + +doc: + - audience: Node Dev + description: | + This PR provides the partial results of the GetRecord kademlia query. + + This significantly improves the authority discovery records, from ~37 minutes to ~2/3 minutes. + In contrast, libp2p discovers authority records in around ~10 minutes. + + The authority discovery was slow because litep2p provided the records only after the Kademlia query was completed. A normal Kademlia query completes in around 40 seconds to a few minutes. + In this PR, partial records are provided as soon as they are discovered from the network. + +crates: + - name: sc-network + bump: patch diff --git a/substrate/client/network/src/litep2p/discovery.rs b/substrate/client/network/src/litep2p/discovery.rs index a48cf867743d..f234d19bedd1 100644 --- a/substrate/client/network/src/litep2p/discovery.rs +++ b/substrate/client/network/src/litep2p/discovery.rs @@ -34,8 +34,8 @@ use litep2p::{ identify::{Config as IdentifyConfig, IdentifyEvent}, kademlia::{ Config as KademliaConfig, ConfigBuilder as KademliaConfigBuilder, - IncomingRecordValidationMode, KademliaEvent, KademliaHandle, QueryId, Quorum, - Record, RecordKey, RecordsType, + IncomingRecordValidationMode, KademliaEvent, KademliaHandle, PeerRecord, QueryId, + Quorum, Record, RecordKey, }, ping::{Config as PingConfig, PingEvent}, }, @@ -129,13 +129,19 @@ pub enum DiscoveryEvent { address: Multiaddr, }, - /// Record was found from the DHT. + /// `GetRecord` query succeeded. GetRecordSuccess { /// Query ID. query_id: QueryId, + }, - /// Records. - records: RecordsType, + /// Record was found from the DHT. + GetRecordPartialResult { + /// Query ID. + query_id: QueryId, + + /// Record. + record: PeerRecord, }, /// Record was successfully stored on the DHT. @@ -550,13 +556,24 @@ impl Stream for Discovery { peers: peers.into_iter().collect(), })) }, - Poll::Ready(Some(KademliaEvent::GetRecordSuccess { query_id, records })) => { + Poll::Ready(Some(KademliaEvent::GetRecordSuccess { query_id })) => { log::trace!( target: LOG_TARGET, - "`GET_RECORD` succeeded for {query_id:?}: {records:?}", + "`GET_RECORD` succeeded for {query_id:?}", ); - return Poll::Ready(Some(DiscoveryEvent::GetRecordSuccess { query_id, records })); + return Poll::Ready(Some(DiscoveryEvent::GetRecordSuccess { query_id })); + }, + Poll::Ready(Some(KademliaEvent::GetRecordPartialResult { query_id, record })) => { + log::trace!( + target: LOG_TARGET, + "`GET_RECORD` intermediary succeeded for {query_id:?}: {record:?}", + ); + + return Poll::Ready(Some(DiscoveryEvent::GetRecordPartialResult { + query_id, + record, + })); }, Poll::Ready(Some(KademliaEvent::PutRecordSuccess { query_id, key: _ })) => return Poll::Ready(Some(DiscoveryEvent::PutRecordSuccess { query_id })), diff --git a/substrate/client/network/src/litep2p/mod.rs b/substrate/client/network/src/litep2p/mod.rs index 8eff3a640b8a..390c2fda69a5 100644 --- a/substrate/client/network/src/litep2p/mod.rs +++ b/substrate/client/network/src/litep2p/mod.rs @@ -59,7 +59,7 @@ use litep2p::{ protocol::{ libp2p::{ bitswap::Config as BitswapConfig, - kademlia::{QueryId, Record, RecordsType}, + kademlia::{QueryId, Record}, }, request_response::ConfigBuilder as RequestResponseConfigBuilder, }, @@ -820,27 +820,45 @@ impl NetworkBackend for Litep2pNetworkBac self.peerstore_handle.add_known_peer(peer.into()); } } - Some(DiscoveryEvent::GetRecordSuccess { query_id, records }) => { - match self.pending_get_values.remove(&query_id) { - None => log::warn!( + Some(DiscoveryEvent::GetRecordPartialResult { query_id, record }) => { + if !self.pending_get_values.contains_key(&query_id) { + log::error!( target: LOG_TARGET, - "`GET_VALUE` succeeded for a non-existent query", - ), + "Missing/invalid pending query for `GET_VALUE` partial result: {query_id:?}" + ); + + continue + } + + let peer_id: sc_network_types::PeerId = record.peer.into(); + let record = PeerRecord { + record: P2PRecord { + key: record.record.key.to_vec().into(), + value: record.record.value, + publisher: record.record.publisher.map(|peer_id| { + let peer_id: sc_network_types::PeerId = peer_id.into(); + peer_id.into() + }), + expires: record.record.expires, + }, + peer: Some(peer_id.into()), + }; + + self.event_streams.send( + Event::Dht( + DhtEvent::ValueFound( + record.into() + ) + ) + ); + } + Some(DiscoveryEvent::GetRecordSuccess { query_id }) => { + match self.pending_get_values.remove(&query_id) { Some((key, started)) => { log::trace!( target: LOG_TARGET, - "`GET_VALUE` for {:?} ({query_id:?}) succeeded", - key, + "`GET_VALUE` for {key:?} ({query_id:?}) succeeded", ); - for record in litep2p_to_libp2p_peer_record(records) { - self.event_streams.send( - Event::Dht( - DhtEvent::ValueFound( - record - ) - ) - ); - } if let Some(ref metrics) = self.metrics { metrics @@ -848,7 +866,14 @@ impl NetworkBackend for Litep2pNetworkBac .with_label_values(&["value-get"]) .observe(started.elapsed().as_secs_f64()); } - } + }, + None => { + log::error!( + target: LOG_TARGET, + "Missing/invalid pending query for `GET_VALUE`: {query_id:?}" + ); + debug_assert!(false); + }, } } Some(DiscoveryEvent::PutRecordSuccess { query_id }) => { @@ -1095,42 +1120,3 @@ impl NetworkBackend for Litep2pNetworkBac } } } - -// Glue code to convert from a litep2p records type to a libp2p2 PeerRecord. -fn litep2p_to_libp2p_peer_record(records: RecordsType) -> Vec { - match records { - litep2p::protocol::libp2p::kademlia::RecordsType::LocalStore(record) => { - vec![PeerRecord { - record: P2PRecord { - key: record.key.to_vec().into(), - value: record.value, - publisher: record.publisher.map(|peer_id| { - let peer_id: sc_network_types::PeerId = peer_id.into(); - peer_id.into() - }), - expires: record.expires, - }, - peer: None, - }] - }, - litep2p::protocol::libp2p::kademlia::RecordsType::Network(records) => records - .into_iter() - .map(|record| { - let peer_id: sc_network_types::PeerId = record.peer.into(); - - PeerRecord { - record: P2PRecord { - key: record.record.key.to_vec().into(), - value: record.record.value, - publisher: record.record.publisher.map(|peer_id| { - let peer_id: sc_network_types::PeerId = peer_id.into(); - peer_id.into() - }), - expires: record.record.expires, - }, - peer: Some(peer_id.into()), - } - }) - .collect::>(), - } -}