-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(swarm): improve PeerAddresses configurability
#5574
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
base: master
Are you sure you want to change the base?
Changes from all commits
e5435fc
3baf1ec
f610b78
7ddd854
4d17c21
457c063
6521a7f
bad6c29
187f010
ad15d3f
b4754f9
38a9c5c
a550d6f
1035649
3e66c4f
8221904
914662d
31ab1fb
312a1f2
3afca6e
632feee
d28be0c
29b2b24
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ name = "libp2p-identify" | |
| edition.workspace = true | ||
| rust-version = { workspace = true } | ||
| description = "Nodes identification protocol for libp2p" | ||
| version = "0.47.0" | ||
| version = "0.48.0" | ||
| authors = ["Parity Technologies <[email protected]>"] | ||
| license = "MIT" | ||
| repository = "https://github.com/libp2p/rust-libp2p" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ name = "libp2p-swarm" | |
| edition.workspace = true | ||
| rust-version = { workspace = true } | ||
| description = "The libp2p swarm" | ||
| version = "0.47.0" | ||
| version = "0.47.1" | ||
| authors = ["Parity Technologies <[email protected]>"] | ||
| license = "MIT" | ||
| repository = "https://github.com/libp2p/rust-libp2p" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,16 +6,58 @@ use libp2p_identity::PeerId; | |
|
|
||
| use crate::{behaviour::FromSwarm, DialError, DialFailure, NewExternalAddrOfPeer}; | ||
|
|
||
| #[derive(Debug, Clone)] | ||
| /// Configuration of a [`PeerAddresses`] instance. | ||
| pub struct PeerAddressesConfig { | ||
| /// Capacity of the [`PeerAddresses`] cache. | ||
| number_of_peers: NonZeroUsize, | ||
|
|
||
| /// Maximum number of cached addresses per peer. | ||
| number_of_addresses_per_peer: NonZeroUsize, | ||
| } | ||
|
|
||
| impl PeerAddressesConfig { | ||
| /// Configure the capacity of the [`PeerAddresses`] cache. | ||
elenaf9 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /// The default capacity is 100. | ||
| pub fn with_number_of_peers(mut self, number_of_peers: NonZeroUsize) -> Self { | ||
| self.number_of_peers = number_of_peers; | ||
| self | ||
| } | ||
|
|
||
| /// Configure the maximum number of cached addresses per peer. | ||
elenaf9 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /// The default number is 10. | ||
| pub fn with_number_of_addresses_per_peer( | ||
| mut self, | ||
| number_of_addresses_per_peer: NonZeroUsize, | ||
| ) -> Self { | ||
| self.number_of_addresses_per_peer = number_of_addresses_per_peer; | ||
| self | ||
| } | ||
| } | ||
|
|
||
| impl Default for PeerAddressesConfig { | ||
| fn default() -> Self { | ||
| Self { | ||
| number_of_peers: NonZeroUsize::new(100).expect("100 != 0"), | ||
dariusc93 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| number_of_addresses_per_peer: NonZeroUsize::new(10).expect("10 != 0"), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Struct for tracking peers' external addresses of the [`Swarm`](crate::Swarm). | ||
| #[derive(Debug)] | ||
| pub struct PeerAddresses(LruCache<PeerId, LruCache<Multiaddr, ()>>); | ||
| pub struct PeerAddresses { | ||
| config: PeerAddressesConfig, | ||
| inner: LruCache<PeerId, LruCache<Multiaddr, ()>>, | ||
| } | ||
|
|
||
| impl PeerAddresses { | ||
| /// Creates a [`PeerAddresses`] cache with capacity for the given number of peers. | ||
| /// | ||
| /// For each peer, we will at most store 10 addresses. | ||
| pub fn new(number_of_peers: NonZeroUsize) -> Self { | ||
| Self(LruCache::new(number_of_peers.get())) | ||
| /// For each peer, we will at most store `config.number_of_addresses_per_peer` addresses. | ||
| pub fn new(config: PeerAddressesConfig) -> Self { | ||
| let inner = LruCache::new(config.number_of_peers.get()); | ||
| Self { config, inner } | ||
|
Comment on lines
+57
to
+60
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah sorry, I just noticed now that this is a breaking change. We could avoid the breaking change by not adding the Wdyt? I understand that by now you might not be working on this anymore, so I can also push this change if folks agree with it. cc @jxs
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi Elena, yeah agree, let's have this as a separate method. |
||
| } | ||
|
|
||
| /// Feed a [`FromSwarm`] event to this struct. | ||
|
|
@@ -47,12 +89,12 @@ impl PeerAddresses { | |
| pub fn add(&mut self, peer: PeerId, address: Multiaddr) -> bool { | ||
| match prepare_addr(&peer, &address) { | ||
| Ok(address) => { | ||
| if let Some(cached) = self.0.get_mut(&peer) { | ||
| if let Some(cached) = self.inner.get_mut(&peer) { | ||
| cached.insert(address, ()).is_none() | ||
| } else { | ||
| let mut set = LruCache::new(10); | ||
| let mut set = LruCache::new(self.config.number_of_addresses_per_peer.get()); | ||
| set.insert(address, ()); | ||
| self.0.insert(peer, set); | ||
| self.inner.insert(peer, set); | ||
|
|
||
| true | ||
| } | ||
|
|
@@ -63,7 +105,7 @@ impl PeerAddresses { | |
|
|
||
| /// Returns peer's external addresses. | ||
| pub fn get(&mut self, peer: &PeerId) -> impl Iterator<Item = Multiaddr> + '_ { | ||
| self.0 | ||
| self.inner | ||
| .get(peer) | ||
| .into_iter() | ||
| .flat_map(|c| c.iter().map(|(m, ())| m)) | ||
|
|
@@ -73,7 +115,7 @@ impl PeerAddresses { | |
| /// Removes address from peer addresses cache. | ||
| /// Returns true if the address was removed. | ||
| pub fn remove(&mut self, peer: &PeerId, address: &Multiaddr) -> bool { | ||
| match self.0.get_mut(peer) { | ||
| match self.inner.get_mut(peer) { | ||
| Some(addrs) => match prepare_addr(peer, address) { | ||
| Ok(address) => addrs.remove(&address).is_some(), | ||
| Err(_) => false, | ||
|
|
@@ -89,7 +131,7 @@ fn prepare_addr(peer: &PeerId, addr: &Multiaddr) -> Result<Multiaddr, Multiaddr> | |
|
|
||
| impl Default for PeerAddresses { | ||
| fn default() -> Self { | ||
| Self(LruCache::new(100)) | ||
| Self::new(Default::default()) | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this required to be optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's
Nonethe cache is disabled. Before, cache size0was used for that.