Consider all peers as potential candidates during pull-request in case of offline nodes (#18333)

* Try all peers during pull-request in case of offline nodes

* fix clippy err

gossip/src/crds_gossip_pull.rs

@@ -18,13 +18,11 @@ use {
         crds_gossip_error::CrdsGossipError,
         crds_value::CrdsValue,
         ping_pong::PingCache,
+        weighted_shuffle::weighted_shuffle,
     },
     itertools::Itertools,
     lru::LruCache,
-    rand::{
-        distributions::{Distribution, WeightedIndex},
-        Rng,
-    },
+    rand::Rng,
     rayon::{prelude::*, ThreadPool},
     solana_runtime::bloom::{AtomicBloom, Bloom},
     solana_sdk::{
@@ -239,10 +237,10 @@ impl CrdsGossipPull {
         }
         let mut peers = {
             let mut rng = rand::thread_rng();
-            let num_samples = peers.len() * 2;
-            let index = WeightedIndex::new(weights).unwrap();
-            let sample_peer = move || peers[index.sample(&mut rng)];
-            repeat_with(sample_peer).take(num_samples)
+            let mut seed = [0u8; 32];
+            rng.fill(&mut seed[..]);
+            let index = weighted_shuffle(&weights, seed);
+            index.into_iter().map(|i| peers[i])
         };
         let peer = {
             let mut rng = rand::thread_rng();
@@ -916,7 +914,7 @@ pub(crate) mod tests {
             &node_keypair.pubkey(),
             0,
         )));
-        let node = CrdsGossipPull::default();
+        let mut node = CrdsGossipPull::default();
         let mut pings = Vec::new();
         let ping_cache = Mutex::new(PingCache::new(
             Duration::from_secs(20 * 60), // ttl
@@ -954,19 +952,20 @@ pub(crate) mod tests {
             ),
             Err(CrdsGossipError::NoPeers)
         );
-        let new = ContactInfo::new_localhost(&solana_sdk::pubkey::new_rand(), 0);
+        let now = 1625029781069;
+        let new = ContactInfo::new_localhost(&solana_sdk::pubkey::new_rand(), now);
         ping_cache
             .lock()
             .unwrap()
             .mock_pong(new.id, new.gossip, Instant::now());
         let new = CrdsValue::new_unsigned(CrdsData::ContactInfo(new));
-        crds.insert(new.clone(), 0).unwrap();
+        crds.insert(new.clone(), now).unwrap();
         let req = node.new_pull_request(
             &thread_pool,
             &crds,
             &node_keypair,
             0,
-            0,
+            now,
             None,
             &HashMap::new(),
             PACKET_DATA_SIZE,
@@ -975,6 +974,27 @@ pub(crate) mod tests {
         );
         let (peer, _) = req.unwrap();
         assert_eq!(peer, *new.contact_info().unwrap());
+
+        node.mark_pull_request_creation_time(new.contact_info().unwrap().id, now);
+        let offline = ContactInfo::new_localhost(&solana_sdk::pubkey::new_rand(), now);
+        let offline = CrdsValue::new_unsigned(CrdsData::ContactInfo(offline));
+        crds.insert(offline, now).unwrap();
+        let req = node.new_pull_request(
+            &thread_pool,
+            &crds,
+            &node_keypair,
+            0,
+            now,
+            None,
+            &HashMap::new(),
+            PACKET_DATA_SIZE,
+            &ping_cache,
+            &mut pings,
+        );
+        // Even though the offline node should have higher weight, we shouldn't request from it
+        // until we receive a ping.
+        let (peer, _) = req.unwrap();
+        assert_eq!(peer, *new.contact_info().unwrap());
     }
 
     #[test]

local-cluster/tests/local_cluster.rs

@@ -2851,14 +2851,12 @@ fn test_hard_fork_invalidates_tower() {
 
 #[test]
 #[serial]
-#[ignore]
 fn test_no_optimistic_confirmation_violation_with_tower() {
     do_test_optimistic_confirmation_violation_with_or_without_tower(true);
 }
 
 #[test]
 #[serial]
-#[ignore]
 fn test_optimistic_confirmation_violation_without_tower() {
     do_test_optimistic_confirmation_violation_with_or_without_tower(false);
 }