Merge #2827

2827: fix: fix peer store evict r=quake,yangby-cryptape a=driftluo

Originally, only the data in the largest group was considered, but now it is changed to traverse at least half of the groups 

Co-authored-by: driftluo <[email protected]>

network/src/peer_store/peer_store_impl.rs

@@ -219,13 +219,19 @@ impl PeerStore {
                     .or_default()
                     .push(addr);
             }
+            let len = peers_by_network_group.len();
+            let mut peers = peers_by_network_group
+                .drain()
+                .map(|(_, v)| v)
+                .collect::<Vec<Vec<_>>>();
+
+            peers.sort_unstable_by_key(|k| std::cmp::Reverse(k.len()));
             let ban_score = self.score_config.ban_score;
-            // find the largest network group
-            peers_by_network_group
-                .values()
-                .max_by_key(|peers| peers.len())
-                .expect("largest network group")
-                .iter()
+            // evict nodes on the same network segment first
+            peers
+                .into_iter()
+                .take(len / 2)
+                .flatten()
                 .filter_map(move |addr| {
                     if addr.is_terrible(now_ms) || addr.score <= ban_score {
                         Some(addr.addr.clone())

network/src/tests/peer_store.rs

@@ -193,8 +193,8 @@ fn test_eviction() {
     let mut peer_store = PeerStore::default();
     let now = faketime::unix_time_as_millis();
     let tried_ms = now - 61_000;
-    // add addrs
-    for i in 0..(ADDR_COUNT_LIMIT - 2) {
+    // add addrs, make the peer store has 4 groups addrs
+    for i in 0..(ADDR_COUNT_LIMIT - 5) {
         let addr: Multiaddr = format!(
             "/ip4/225.0.0.1/tcp/{}/p2p/{}",
             i,
@@ -204,27 +204,47 @@ fn test_eviction() {
         .unwrap();
         peer_store.add_addr(addr).unwrap();
     }
+    let addr: Multiaddr = format!(
+        "/ip4/192.163.1.1/tcp/43/p2p/{}",
+        PeerId::random().to_base58()
+    )
+    .parse()
+    .unwrap();
+    peer_store.add_addr(addr).unwrap();
+    let addr: Multiaddr = format!(
+        "/ip4/255.255.0.1/tcp/43/p2p/{}",
+        PeerId::random().to_base58()
+    )
+    .parse()
+    .unwrap();
+    peer_store.add_addr(addr).unwrap();
+    let addr: Multiaddr = format!("/ip4/239.0.0.1/tcp/43/p2p/{}", PeerId::random().to_base58())
+        .parse()
+        .unwrap();
+    peer_store.add_addr(addr).unwrap();
+
     // this peer will be evict from peer store
     let evict_addr: Multiaddr =
         format!("/ip4/225.0.0.2/tcp/42/p2p/{}", PeerId::random().to_base58())
             .parse()
             .unwrap();
     peer_store.add_addr(evict_addr.clone()).unwrap();
-    // this peer will reserve in peer store
-    let reserved_addr: Multiaddr = format!(
+    // this peer will be evict from peer store
+    let evict_addr_2: Multiaddr = format!(
         "/ip4/192.163.1.1/tcp/42/p2p/{}",
         PeerId::random().to_base58()
     )
     .parse()
     .unwrap();
-    peer_store.add_addr(reserved_addr.clone()).unwrap();
+    peer_store.add_addr(evict_addr_2.clone()).unwrap();
     // mark two peers as terrible peer
     if let Some(paddr) = peer_store.mut_addr_manager().get_mut(&evict_addr) {
         paddr.mark_tried(tried_ms);
         paddr.mark_tried(tried_ms);
         paddr.mark_tried(tried_ms);
+        assert!(paddr.is_terrible(now));
     }
-    if let Some(paddr) = peer_store.mut_addr_manager().get_mut(&reserved_addr) {
+    if let Some(paddr) = peer_store.mut_addr_manager().get_mut(&evict_addr_2) {
         paddr.mark_tried(tried_ms);
         paddr.mark_tried(tried_ms);
         paddr.mark_tried(tried_ms);
@@ -237,9 +257,9 @@ fn test_eviction() {
             .unwrap();
     peer_store.add_addr(new_peer_addr.clone()).unwrap();
     // check addrs
-    // peer store will only evict peers from largest network group
-    // the evict_addr should be evict, other two addrs will remain in peer store
+    // peer store will evict peers from largest network group to low group
+    // the two evict_addrs should be evict, other addrs will remain in peer store
     assert!(peer_store.mut_addr_manager().get(&new_peer_addr).is_some());
-    assert!(peer_store.mut_addr_manager().get(&reserved_addr).is_some());
+    assert!(peer_store.mut_addr_manager().get(&evict_addr_2).is_none());
     assert!(peer_store.mut_addr_manager().get(&evict_addr).is_none());
 }

util/app-config/src/configs/memory_tracker.rs

@@ -1,8 +1,8 @@
 use serde::{Deserialize, Serialize};
 
 /// Memory tracker config options.
-#[serde(deny_unknown_fields)]
 #[derive(Clone, Debug, Serialize, Deserialize)]
+#[serde(deny_unknown_fields)]
 pub struct Config {
     /// Tracking interval in seconds.
     pub interval: u64,