protocols/kad: Rework debug log and doc comments

libp2p · Jun 17, 2020 · 1232c55 · 1232c55
1 parent 0c964ef
commit 1232c55
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 30 deletions.
diff --git a/protocols/kad/src/behaviour.rs b/protocols/kad/src/behaviour.rs
@@ -1511,24 +1511,22 @@ where
                         if let Some(record) = record {
                             records.push(PeerRecord{ peer: Some(source.clone()), record });
 
-                            if records.len() >= quorum.get() {
+                            let quorum = quorum.get();
+                            if records.len() >= quorum {
                                 // Desired quorum reached. The query may finish. See
                                 // [`Query::try_finish`] for details.
                                 let peers = records.iter()
                                     .filter_map(|PeerRecord{ peer, .. }| peer.as_ref())
                                     .cloned()
                                     .collect::<Vec<_>>();
-                                let is_finished = query.try_finish(peers.iter());
-
-                                debug!(
-                                    "GetRecord query with id {:?} reached the quorum {} finished \
-                                     with recent response from peer {}, counting {} successful \
-                                     responses total.",
-                                    user_data,
-                                    if is_finished { "and is" } else { "but is not yet" },
-                                    source,
-                                    peers.len(),
-                                );
+                                let finished = query.try_finish(peers.iter());
+                                if !finished {
+                                    debug!(
+                                        "GetRecord query ({:?}) reached quorum ({}/{}) with \
+                                         response from peer {} but could not yet finish.",
+                                        user_data, peers.len(), quorum, source,
+                                    );
+                                }
                             }
                         } else if quorum.get() == 1 {
                             // It is a "standard" Kademlia query, for which the
@@ -1567,19 +1565,18 @@ where
                         phase: PutRecordPhase::PutRecord { success, .. }, quorum, ..
                     } = &mut query.inner.info {
                         success.push(source.clone());
-                        if success.len() >= quorum.get() {
+
+                        let quorum = quorum.get();
+                        if success.len() >= quorum {
                             let peers = success.clone();
-                            let is_finished = query.try_finish(peers.iter());
-
-                            debug!(
-                                "PutRecord query with id {:?} reached the quorum {} finished \
-                                 with recent response from peer {}, counting {} successful \
-                                 responses total.",
-                                user_data,
-                                if is_finished { "and is" } else { "but is not yet" },
-                                source,
-                                peers.len(),
-                            );
+                            let finished = query.try_finish(peers.iter());
+                            if !finished {
+                                debug!(
+                                    "PutRecord query ({:?}) reached quorum ({}/{}) with response \
+                                     from peer {} but could not yet finish.",
+                                    user_data, peers.len(), quorum, source,
+                                );
+                            }
                         }
                     }
                 }

diff --git a/protocols/kad/src/query.rs b/protocols/kad/src/query.rs
@@ -351,6 +351,10 @@ impl<TInner> Query<TInner> {
     /// which this is considered to be the case, i.e. for which a termination
     /// condition is satisfied.
     ///
+    /// Returns `true` if the query did indeed finish, `false` otherwise. In the
+    /// latter case, a new attempt at finishing the query may be made with new
+    /// `peers`.
+    ///
     /// A finished query immediately stops yielding new peers to contact and
     /// will be reported by [`QueryPool::poll`] via
     /// [`QueryPoolState::Finished`].
@@ -359,12 +363,10 @@ impl<TInner> Query<TInner> {
         I: IntoIterator<Item = &'a PeerId>
     {
         match &mut self.peer_iter {
-            QueryPeerIter::Closest(iter) => iter.finish(),
-            QueryPeerIter::ClosestDisjoint(iter) => return iter.finish_paths(peers),
-            QueryPeerIter::Fixed(iter) => iter.finish()
-        };
-
-        true
+            QueryPeerIter::Closest(iter) => { iter.finish(); true },
+            QueryPeerIter::ClosestDisjoint(iter) => iter.finish_paths(peers),
+            QueryPeerIter::Fixed(iter) => { iter.finish(); true }
+        }
     }
 
     /// Finishes the query prematurely.