Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Commit d4b30ad

Browse files
behzadnouribehzadnouri
authored
reworks gossip crds timeouts (#30468)
CrdsGossipPull::make_timeouts iterates over the stakes hashmap and creates a new hashmap which is unnecessary: https://github.com/solana-labs/solana/blob/c032dc275/gossip/src/crds_gossip_pull.rs#L517-L539 The commit instead keeps a reference to the stakes hashmap.
1 parent b53656b commit d4b30ad

File tree

6 files changed

+209
-99
lines changed

6 files changed

+209
-99
lines changed

‎gossip/benches/crds.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ use {
77
rayon::ThreadPoolBuilder,
88
solana_gossip::{
99
crds::{Crds, GossipRoute},
10-
crds_gossip_pull::CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS,
10+
crds_gossip_pull::{CrdsTimeouts, CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS},
1111
crds_value::CrdsValue,
1212
},
1313
solana_sdk::pubkey::Pubkey,
14-
std::collections::HashMap,
14+
std::{collections::HashMap, time::Duration},
1515
test::Bencher,
1616
};
1717

@@ -24,8 +24,13 @@ fn bench_find_old_labels(bencher: &mut Bencher) {
2424
std::iter::repeat_with(|| (CrdsValue::new_rand(&mut rng, None), rng.gen_range(0, now)))
2525
.take(50_000)
2626
.for_each(|(v, ts)| assert!(crds.insert(v, ts, GossipRoute::LocalMessage).is_ok()));
27-
let mut timeouts = HashMap::new();
28-
timeouts.insert(Pubkey::default(), CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS);
27+
let stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
28+
let timeouts = CrdsTimeouts::new(
29+
Pubkey::new_unique(),
30+
CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS, // default_timeout
31+
Duration::from_secs(48 * 3600), // epoch_duration
32+
&stakes,
33+
);
2934
bencher.iter(|| {
3035
let out = crds.find_old_labels(&thread_pool, now, &timeouts);
3136
assert!(out.len() > 10);

‎gossip/src/cluster_info.rs

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ use {
2828
crds::{Crds, Cursor, GossipRoute},
2929
crds_gossip::CrdsGossip,
3030
crds_gossip_error::CrdsGossipError,
31-
crds_gossip_pull::{CrdsFilter, ProcessPullStats, CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS},
31+
crds_gossip_pull::{
32+
CrdsFilter, CrdsTimeouts, ProcessPullStats, CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS,
33+
},
3234
crds_value::{
3335
self, CrdsData, CrdsValue, CrdsValueLabel, EpochSlotsIndex, IncrementalSnapshotHashes,
3436
LowestSlot, NodeInstance, SnapshotHashes, Version, Vote, MAX_WALLCLOCK,
@@ -2158,7 +2160,7 @@ impl ClusterInfo {
21582160
&self,
21592161
from: &Pubkey,
21602162
crds_values: Vec<CrdsValue>,
2161-
timeouts: &HashMap<Pubkey, u64>,
2163+
timeouts: &CrdsTimeouts,
21622164
) -> (usize, usize, usize) {
21632165
let len = crds_values.len();
21642166
trace!("PullResponse me: {} from: {} len={}", self.id(), from, len);
@@ -3305,9 +3307,13 @@ RPC Enabled Nodes: 1"#;
33053307
});
33063308
let entrypoint_pubkey = solana_sdk::pubkey::new_rand();
33073309
let data = test_crds_values(entrypoint_pubkey);
3308-
let timeouts = [(Pubkey::default(), CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS)]
3309-
.into_iter()
3310-
.collect();
3310+
let stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
3311+
let timeouts = CrdsTimeouts::new(
3312+
cluster_info.id(),
3313+
CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS, // default_timeout
3314+
Duration::from_secs(48 * 3600), // epoch_duration
3315+
&stakes,
3316+
);
33113317
assert_eq!(
33123318
(0, 0, 1),
33133319
ClusterInfo::handle_pull_response(
@@ -4081,9 +4087,10 @@ RPC Enabled Nodes: 1"#;
40814087
let entrypoint_crdsvalue =
40824088
CrdsValue::new_unsigned(CrdsData::LegacyContactInfo(entrypoint.clone()));
40834089
let cluster_info = Arc::new(cluster_info);
4090+
let stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
40844091
let timeouts = cluster_info.gossip.make_timeouts(
40854092
cluster_info.id(),
4086-
&HashMap::default(), // stakes,
4093+
&stakes,
40874094
Duration::from_millis(cluster_info.gossip.pull.crds_timeout),
40884095
);
40894096
ClusterInfo::handle_pull_response(
@@ -4729,8 +4736,13 @@ RPC Enabled Nodes: 1"#;
47294736
})
47304737
.take(NO_ENTRIES)
47314738
.collect();
4732-
let mut timeouts = HashMap::new();
4733-
timeouts.insert(Pubkey::default(), CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS * 4);
4739+
let stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
4740+
let timeouts = CrdsTimeouts::new(
4741+
cluster_info.id(),
4742+
CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS * 4, // default_timeout
4743+
Duration::from_secs(48 * 3600), // epoch_duration
4744+
&stakes,
4745+
);
47344746
assert_eq!(
47354747
(0, 0, NO_ENTRIES),
47364748
cluster_info.handle_pull_response(&entrypoint_pubkey, data, &timeouts)

‎gossip/src/crds.rs

Lines changed: 102 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
use {
2828
crate::{
2929
crds_entry::CrdsEntry,
30+
crds_gossip_pull::CrdsTimeouts,
3031
crds_shards::CrdsShards,
3132
crds_value::{CrdsData, CrdsValue, CrdsValueLabel},
3233
legacy_contact_info::LegacyContactInfo as ContactInfo,
@@ -472,15 +473,12 @@ impl Crds {
472473
&self,
473474
thread_pool: &ThreadPool,
474475
now: u64,
475-
timeouts: &HashMap<Pubkey, u64>,
476+
timeouts: &CrdsTimeouts,
476477
) -> Vec<CrdsValueLabel> {
477-
let default_timeout = *timeouts
478-
.get(&Pubkey::default())
479-
.expect("must have default timeout");
480478
// Given an index of all crd values associated with a pubkey,
481479
// returns crds labels of old values to be evicted.
482480
let evict = |pubkey, index: &IndexSet<usize>| {
483-
let timeout = timeouts.get(pubkey).copied().unwrap_or(default_timeout);
481+
let timeout = timeouts[pubkey];
484482
// If the origin's contact-info hasn't expired yet then preserve
485483
// all associated values.
486484
let origin = CrdsValueLabel::LegacyContactInfo(*pubkey);
@@ -732,7 +730,7 @@ mod tests {
732730
signature::{Keypair, Signer},
733731
timing::timestamp,
734732
},
735-
std::{collections::HashSet, iter::repeat_with, net::Ipv4Addr},
733+
std::{collections::HashSet, iter::repeat_with, net::Ipv4Addr, time::Duration},
736734
};
737735

738736
#[test]
@@ -888,17 +886,34 @@ mod tests {
888886
crds.insert(val.clone(), 1, GossipRoute::LocalMessage),
889887
Ok(())
890888
);
891-
let mut set = HashMap::new();
892-
set.insert(Pubkey::default(), 0);
893-
assert!(crds.find_old_labels(&thread_pool, 0, &set).is_empty());
894-
set.insert(Pubkey::default(), 1);
889+
let pubkey = Pubkey::new_unique();
890+
let stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
891+
let epoch_duration = Duration::from_secs(48 * 3600);
892+
let timeouts = CrdsTimeouts::new(
893+
pubkey,
894+
0u64, // default_timeout,
895+
epoch_duration,
896+
&stakes,
897+
);
898+
assert!(crds.find_old_labels(&thread_pool, 0, &timeouts).is_empty());
899+
let timeouts = CrdsTimeouts::new(
900+
pubkey,
901+
1u64, // default_timeout,
902+
epoch_duration,
903+
&stakes,
904+
);
895905
assert_eq!(
896-
crds.find_old_labels(&thread_pool, 2, &set),
906+
crds.find_old_labels(&thread_pool, 2, &timeouts),
897907
vec![val.label()]
898908
);
899-
set.insert(Pubkey::default(), 2);
909+
let timeouts = CrdsTimeouts::new(
910+
pubkey,
911+
2u64, // default_timeout,
912+
epoch_duration,
913+
&stakes,
914+
);
900915
assert_eq!(
901-
crds.find_old_labels(&thread_pool, 4, &set),
916+
crds.find_old_labels(&thread_pool, 4, &timeouts),
902917
vec![val.label()]
903918
);
904919
}
@@ -907,24 +922,51 @@ mod tests {
907922
let thread_pool = ThreadPoolBuilder::new().build().unwrap();
908923
let mut rng = thread_rng();
909924
let mut crds = Crds::default();
910-
let mut timeouts = HashMap::new();
911925
let val = CrdsValue::new_rand(&mut rng, None);
912-
timeouts.insert(Pubkey::default(), 3);
926+
let mut stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
927+
let timeouts = CrdsTimeouts::new(
928+
Pubkey::new_unique(),
929+
3, // default_timeout
930+
Duration::from_secs(48 * 3600), // epoch_duration
931+
&stakes,
932+
);
913933
assert_eq!(
914934
crds.insert(val.clone(), 0, GossipRoute::LocalMessage),
915935
Ok(())
916936
);
917937
assert!(crds.find_old_labels(&thread_pool, 2, &timeouts).is_empty());
918-
timeouts.insert(val.pubkey(), 1);
938+
stakes.insert(val.pubkey(), 1u64);
939+
let timeouts = CrdsTimeouts::new(
940+
Pubkey::new_unique(),
941+
1, // default_timeout
942+
Duration::from_millis(1), // epoch_duration
943+
&stakes,
944+
);
919945
assert_eq!(
920946
crds.find_old_labels(&thread_pool, 2, &timeouts),
921947
vec![val.label()]
922948
);
923-
timeouts.insert(val.pubkey(), u64::MAX);
949+
let timeouts = CrdsTimeouts::new(
950+
Pubkey::new_unique(),
951+
3, // default_timeout
952+
Duration::from_secs(48 * 3600), // epoch_duration
953+
&stakes,
954+
);
924955
assert!(crds.find_old_labels(&thread_pool, 2, &timeouts).is_empty());
925-
timeouts.insert(Pubkey::default(), 1);
956+
let timeouts = CrdsTimeouts::new(
957+
Pubkey::new_unique(),
958+
1, // default_timeout
959+
Duration::from_secs(48 * 3600), // epoch_duration
960+
&stakes,
961+
);
926962
assert!(crds.find_old_labels(&thread_pool, 2, &timeouts).is_empty());
927-
timeouts.remove(&val.pubkey());
963+
stakes.remove(&val.pubkey());
964+
let timeouts = CrdsTimeouts::new(
965+
Pubkey::new_unique(),
966+
1, // default_timeout
967+
Duration::from_secs(48 * 3600), // epoch_duration
968+
&stakes,
969+
);
928970
assert_eq!(
929971
crds.find_old_labels(&thread_pool, 2, &timeouts),
930972
vec![val.label()]
@@ -940,14 +982,19 @@ mod tests {
940982
crds.insert(val.clone(), 1, GossipRoute::LocalMessage),
941983
Ok(_)
942984
);
943-
let mut set = HashMap::new();
944-
set.insert(Pubkey::default(), 1);
985+
let stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
986+
let timeouts = CrdsTimeouts::new(
987+
Pubkey::new_unique(),
988+
1, // default_timeout
989+
Duration::from_secs(48 * 3600), // epoch_duration
990+
&stakes,
991+
);
945992
assert_eq!(
946-
crds.find_old_labels(&thread_pool, 2, &set),
993+
crds.find_old_labels(&thread_pool, 2, &timeouts),
947994
vec![val.label()]
948995
);
949996
crds.remove(&val.label(), /*now=*/ 0);
950-
assert!(crds.find_old_labels(&thread_pool, 2, &set).is_empty());
997+
assert!(crds.find_old_labels(&thread_pool, 2, &timeouts).is_empty());
951998
}
952999
#[test]
9531000
fn test_find_old_records_staked() {
@@ -961,28 +1008,35 @@ mod tests {
9611008
crds.insert(val.clone(), 1, GossipRoute::LocalMessage),
9621009
Ok(())
9631010
);
964-
let mut set = HashMap::new();
1011+
let mut stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
1012+
let timeouts = CrdsTimeouts::new(
1013+
Pubkey::new_unique(),
1014+
0, // default_timeout
1015+
Duration::from_secs(48 * 3600), // epoch_duration
1016+
&stakes,
1017+
);
9651018
//now < timestamp
966-
set.insert(Pubkey::default(), 0);
967-
set.insert(val.pubkey(), 0);
968-
assert!(crds.find_old_labels(&thread_pool, 0, &set).is_empty());
1019+
assert!(crds.find_old_labels(&thread_pool, 0, &timeouts).is_empty());
9691020

9701021
//pubkey shouldn't expire since its timeout is MAX
971-
set.insert(val.pubkey(), std::u64::MAX);
972-
assert!(crds.find_old_labels(&thread_pool, 2, &set).is_empty());
973-
974-
//default has max timeout, but pubkey should still expire
975-
set.insert(Pubkey::default(), std::u64::MAX);
976-
set.insert(val.pubkey(), 1);
977-
assert_eq!(
978-
crds.find_old_labels(&thread_pool, 2, &set),
979-
vec![val.label()]
1022+
stakes.insert(val.pubkey(), 1u64);
1023+
let timeouts = CrdsTimeouts::new(
1024+
Pubkey::new_unique(),
1025+
0, // default_timeout
1026+
Duration::from_secs(48 * 3600), // epoch_duration
1027+
&stakes,
9801028
);
1029+
assert!(crds.find_old_labels(&thread_pool, 2, &timeouts).is_empty());
9811030

982-
set.insert(val.pubkey(), 2);
983-
assert!(crds.find_old_labels(&thread_pool, 2, &set).is_empty());
1031+
let timeouts = CrdsTimeouts::new(
1032+
Pubkey::new_unique(),
1033+
0, // default_timeout
1034+
Duration::from_millis(2), // epoch_duration
1035+
&stakes,
1036+
);
1037+
assert!(crds.find_old_labels(&thread_pool, 2, &timeouts).is_empty());
9841038
assert_eq!(
985-
crds.find_old_labels(&thread_pool, 3, &set),
1039+
crds.find_old_labels(&thread_pool, 3, &timeouts),
9861040
vec![val.label()]
9871041
);
9881042
}
@@ -1353,17 +1407,19 @@ mod tests {
13531407
crds.insert(val.clone(), 1, GossipRoute::LocalMessage),
13541408
Ok(_)
13551409
);
1356-
let mut set = HashMap::new();
1357-
1358-
//default has max timeout, but pubkey should still expire
1359-
set.insert(Pubkey::default(), std::u64::MAX);
1360-
set.insert(val.pubkey(), 1);
1410+
let stakes = HashMap::from([(Pubkey::new_unique(), 1u64)]);
1411+
let timeouts = CrdsTimeouts::new(
1412+
Pubkey::new_unique(),
1413+
1, // default_timeout
1414+
Duration::from_millis(1), // epoch_duration
1415+
&stakes,
1416+
);
13611417
assert_eq!(
1362-
crds.find_old_labels(&thread_pool, 2, &set),
1418+
crds.find_old_labels(&thread_pool, 2, &timeouts),
13631419
vec![val.label()]
13641420
);
13651421
crds.remove(&val.label(), /*now=*/ 0);
1366-
assert!(crds.find_old_labels(&thread_pool, 2, &set).is_empty());
1422+
assert!(crds.find_old_labels(&thread_pool, 2, &timeouts).is_empty());
13671423
}
13681424

13691425
#[test]

‎gossip/src/crds_gossip.rs

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use {
1010
cluster_info_metrics::GossipStats,
1111
crds::{Crds, GossipRoute},
1212
crds_gossip_error::CrdsGossipError,
13-
crds_gossip_pull::{CrdsFilter, CrdsGossipPull, ProcessPullStats},
13+
crds_gossip_pull::{CrdsFilter, CrdsGossipPull, CrdsTimeouts, ProcessPullStats},
1414
crds_gossip_push::CrdsGossipPush,
1515
crds_value::{CrdsData, CrdsValue},
1616
duplicate_shred::{self, DuplicateShredIndex, MAX_DUPLICATE_SHREDS},
@@ -258,7 +258,7 @@ impl CrdsGossip {
258258

259259
pub fn filter_pull_responses(
260260
&self,
261-
timeouts: &HashMap<Pubkey, u64>,
261+
timeouts: &CrdsTimeouts,
262262
response: Vec<CrdsValue>,
263263
now: u64,
264264
process_pull_stats: &mut ProcessPullStats,
@@ -292,12 +292,12 @@ impl CrdsGossip {
292292
);
293293
}
294294

295-
pub fn make_timeouts(
295+
pub fn make_timeouts<'a>(
296296
&self,
297297
self_pubkey: Pubkey,
298-
stakes: &HashMap<Pubkey, u64>,
298+
stakes: &'a HashMap<Pubkey, u64>,
299299
epoch_duration: Duration,
300-
) -> HashMap<Pubkey, u64> {
300+
) -> CrdsTimeouts<'a> {
301301
self.pull.make_timeouts(self_pubkey, stakes, epoch_duration)
302302
}
303303

@@ -306,13 +306,12 @@ impl CrdsGossip {
306306
self_pubkey: &Pubkey,
307307
thread_pool: &ThreadPool,
308308
now: u64,
309-
timeouts: &HashMap<Pubkey, u64>,
309+
timeouts: &CrdsTimeouts,
310310
) -> usize {
311311
let mut rv = 0;
312312
if now > self.pull.crds_timeout {
313-
//sanity check
314-
assert_eq!(timeouts[self_pubkey], std::u64::MAX);
315-
assert!(timeouts.contains_key(&Pubkey::default()));
313+
debug_assert_eq!(timeouts[self_pubkey], u64::MAX);
314+
debug_assert_ne!(timeouts[&Pubkey::default()], 0u64);
316315
rv = CrdsGossipPull::purge_active(thread_pool, &self.crds, now, timeouts);
317316
}
318317
self.crds

0 commit comments

Comments
 (0)