From 49967514c21ac2f4579a7c4c8b85f10a79db14f5 Mon Sep 17 00:00:00 2001 From: Kim Altintop Date: Thu, 19 Aug 2021 10:19:44 +0200 Subject: [PATCH] seed: consider if tracking relationship was already established When calling `Node::track_project`, it might very well be that the project/peer is already tracked, which would result in unnecessary broadcast traffic. Thus, don't emit any events in this case. Also, change the order of `track` and `replicate` to ensure we do actually fetch the newly-tracked peer's view. Additionally, we can skip `replicate` if we already tracked the peer, as subsequent broadcast messages won't be considered uninteresting. Ref #734 Signed-off-by: Kim Altintop --- seed/src/lib.rs | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/seed/src/lib.rs b/seed/src/lib.rs index 10e61c8d2..d7928f1f6 100644 --- a/seed/src/lib.rs +++ b/seed/src/lib.rs @@ -399,7 +399,7 @@ impl Node { if mode.is_trackable(peer_id, urn) { // Attempt to track, but keep going if it fails. - if Node::track_project(api, urn, provider).await.is_ok() { + if let Ok(true) = Node::track_project(api, urn, provider).await { let event = Event::project_tracked(urn.clone(), *peer_id, api).await?; api.announce(Payload { urn: urn.clone(), @@ -425,11 +425,14 @@ impl Node { } /// Attempt to track a project. + /// + /// Returns `false` if the project was already tracked (so this call had no + /// effect), `true` otherwise. async fn track_project( api: &Peer, urn: &Urn, peer_info: &PeerInfo, - ) -> Result<(), Error> { + ) -> Result { let peer_id = peer_info.peer_id; let addr_hints = peer_info.seen_addrs.iter().copied().collect::>(); @@ -440,18 +443,25 @@ impl Node { let fetcher = fetcher::PeerToPeer::new(urn.clone(), peer_id, addr_hints) .build(storage) .map_err(|e| Error::MkFetcher(e.into()))??; - replication::replicate(storage, fetcher, cfg, None)?; - tracking::track(storage, &urn, peer_id)?; + let was_updated = tracking::track(storage, &urn, peer_id)?; + // Skip explicit replication if we already track the peer -- + // normal gossip should take care of that. + if was_updated { + replication::replicate(storage, fetcher, cfg, None)?; + } - Ok::<_, Error>(()) + Ok::<_, Error>(was_updated) }) .await? }; match &result { - Ok(()) => { + Ok(true) => { tracing::info!("Successfully tracked project {} from peer {}", urn, peer_id); }, + Ok(false) => { + tracing::debug!("Project {} from peer {} already tracked", urn, peer_id); + }, Err(err) => { tracing::info!( "Error tracking project {} from peer {}: {}",