-
Notifications
You must be signed in to change notification settings - Fork 11
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
kad: Providers part 6: stop providing #245
Merged
+74
−11
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
1048320
Introduce `ADD_PROVIDER` query
dmitry-markin 2ab84ae
Merge remote-tracking branch 'origin/master' into dm-provider-queries
dmitry-markin 778078b
Execute `ADD_PROVIDER` query
dmitry-markin 323a26f
Introduce local providers in `MemoryStore`
dmitry-markin 8946809
Add provider refresh interval to Kademlia config
dmitry-markin 474a58a
Merge remote-tracking branch 'origin/master' into dm-republish-providers
dmitry-markin d02acee
Move `FuturesStream` to a separate file
dmitry-markin 5432c9b
Refresh providers: dry-run without network queries
dmitry-markin feb493d
Remove `try_get_record()` and other `try_...()` non-async methods
dmitry-markin 982c73d
Move query ID generation from `KademliaHandle` to `Kademlia`
dmitry-markin 68c7a87
Republish providers
dmitry-markin f9ed3e7
Merge remote-tracking branch 'origin/master' into dm-republish-providers
dmitry-markin 764a3f2
Merge remote-tracking branch 'origin/master' into dm-add-providers
dmitry-markin 5e97484
Use getter `TransportService::local_peer_id()` instead of accessing d…
dmitry-markin 7caf290
Use getter `TransportService::local_peer_id()` instead of accessing d…
dmitry-markin 60b8fb6
Introduce `GET_PROVIDERS` query
dmitry-markin d340c35
Update `get_providers()` to receive query ID via oneshot channel
dmitry-markin 10b96db
Make lines fit into 100 characters
dmitry-markin 6146540
Introduce `GetProvidersContext` & `GetProvidersConfig`
dmitry-markin e167419
Implement `GET_PROVIDERS` query
dmitry-markin eb6eee8
Merge returned provider addresses and deduplicate records
dmitry-markin 8314d95
Merge branch 'dm-add-providers' into dm-republish-providers
dmitry-markin 974eac5
minor: fix log target
dmitry-markin e895a44
Revert "Remove `try_get_record()` and other `try_...()` non-async met…
dmitry-markin ce18594
Revert "Move query ID generation from `KademliaHandle` to `Kademlia`"
dmitry-markin 0fcd621
Use `AtomicUsize` to generate `QueryId` in both `KademliaHandle` and …
dmitry-markin 5f26161
Merge branch 'dm-republish-providers' into dm-get-providers
dmitry-markin 57f17ce
Use `open_substream_or_dial()` to add provider records to peers
dmitry-markin 8594103
Merge remote-tracking branch 'origin/master' into dm-add-providers
dmitry-markin 4861959
Fix refresh when we are the only provider
dmitry-markin c85dd3b
Merge branch 'dm-republish-providers' into dm-get-providers
dmitry-markin 03846c7
Address review suggestions
dmitry-markin 7492895
Stop providing
dmitry-markin c90861f
Use `HashMap::entry()` API to remove local providers
dmitry-markin e853d3b
Merge branch 'dm-add-providers' into dm-republish-providers
dmitry-markin aeb59cf
Merge branch 'dm-republish-providers' into dm-get-providers
dmitry-markin 674c54e
Merge branch 'dm-get-providers' into dm-stop-providing
dmitry-markin d6386ac
Merge remote-tracking branch 'origin/master' into dm-stop-providing
dmitry-markin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,20 +20,19 @@ | |
|
||
//! Memory store implementation for Kademlia. | ||
|
||
#![allow(unused)] | ||
use crate::{ | ||
protocol::libp2p::kademlia::{ | ||
config::DEFAULT_PROVIDER_REFRESH_INTERVAL, | ||
futures_stream::FuturesStream, | ||
record::{Key, ProviderRecord, Record}, | ||
types::Key as KademliaKey, | ||
}, | ||
PeerId, | ||
}; | ||
|
||
use futures::{future::BoxFuture, stream::FuturesUnordered, StreamExt}; | ||
use futures::{future::BoxFuture, StreamExt}; | ||
use std::{ | ||
collections::{hash_map::Entry, HashMap}, | ||
num::NonZeroUsize, | ||
time::Duration, | ||
}; | ||
|
||
|
@@ -63,6 +62,7 @@ pub struct MemoryStore { | |
|
||
impl MemoryStore { | ||
/// Create new [`MemoryStore`]. | ||
#[cfg(test)] | ||
pub fn new(local_peer_id: PeerId) -> Self { | ||
Self { | ||
local_peer_id, | ||
|
@@ -149,14 +149,14 @@ impl MemoryStore { | |
/// | ||
/// Returns a non-empty list of providers, if any. | ||
pub fn get_providers(&mut self, key: &Key) -> Vec<ProviderRecord> { | ||
let drop = self.provider_keys.get_mut(key).map_or(false, |providers| { | ||
let drop_key = self.provider_keys.get_mut(key).map_or(false, |providers| { | ||
let now = std::time::Instant::now(); | ||
providers.retain(|p| !p.is_expired(now)); | ||
|
||
providers.is_empty() | ||
}); | ||
|
||
if drop { | ||
if drop_key { | ||
self.provider_keys.remove(key); | ||
|
||
Vec::default() | ||
|
@@ -211,7 +211,7 @@ impl MemoryStore { | |
false | ||
}, | ||
Entry::Occupied(mut entry) => { | ||
let mut providers = entry.get_mut(); | ||
let providers = entry.get_mut(); | ||
|
||
// Providers under every key are sorted by distance from the provided key, with | ||
// equal distances meaning peer IDs (more strictly, their hashes) | ||
|
@@ -261,6 +261,46 @@ impl MemoryStore { | |
} | ||
} | ||
|
||
/// Remove local provider for `key`. | ||
pub fn remove_local_provider(&mut self, key: Key) { | ||
if self.local_providers.remove(&key).is_none() { | ||
tracing::warn!(?key, "trying to remove nonexistent local provider",); | ||
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. nit: Could remove the trailing comma here and below? |
||
return; | ||
}; | ||
|
||
match self.provider_keys.entry(key.clone()) { | ||
Entry::Vacant(_) => { | ||
tracing::error!(?key, "local provider key not found during removal",); | ||
debug_assert!(false); | ||
return; | ||
} | ||
Entry::Occupied(mut entry) => { | ||
let providers = entry.get_mut(); | ||
|
||
// Providers are sorted by distance. | ||
let local_provider_distance = KademliaKey::from(self.local_peer_id.clone()) | ||
.distance(&KademliaKey::new(key.clone())); | ||
let provider_position = | ||
providers.binary_search_by(|p| p.distance().cmp(&local_provider_distance)); | ||
|
||
match provider_position { | ||
Ok(i) => { | ||
providers.remove(i); | ||
} | ||
Err(_) => { | ||
tracing::error!(?key, "local provider not found during removal",); | ||
debug_assert!(false); | ||
return; | ||
} | ||
} | ||
|
||
if providers.is_empty() { | ||
entry.remove(); | ||
} | ||
} | ||
}; | ||
} | ||
|
||
/// Poll next action from the store. | ||
pub async fn next_action(&mut self) -> Option<MemoryStoreAction> { | ||
// [`FuturesStream`] never terminates, so `map()` below is always triggered. | ||
|
@@ -328,10 +368,7 @@ impl Default for MemoryStoreConfig { | |
mod tests { | ||
use super::*; | ||
use crate::PeerId; | ||
use multiaddr::{ | ||
multiaddr, | ||
Protocol::{Ip4, Tcp}, | ||
}; | ||
use multiaddr::multiaddr; | ||
|
||
#[test] | ||
fn put_get_record() { | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
dq: Do you think we should consider an eviction strategy from the memory store that removes older but not yet expired records similar to LRU?