From b7dc7d859cf5bcff17802b741ec6c205491bc7df Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Mon, 24 May 2021 20:10:44 +0000 Subject: [PATCH] removes Crds::lookup and lookup_versioned (backport #17438) (#17441) * removes Crds::lookup and lookup_versioned (#17438) (cherry picked from commit e867d7f3b8f77558649eb0f392b49e966c165d16) * patches push_epoch_slots for backport Co-authored-by: behzad nouri --- core/src/cluster_info.rs | 51 ++++++++++++++---------------------- core/src/crds.rs | 8 ------ core/src/crds_gossip_pull.rs | 50 +++++++++++------------------------ core/src/crds_gossip_push.rs | 4 +-- core/tests/crds_gossip.rs | 20 +++++--------- 5 files changed, 44 insertions(+), 89 deletions(-) diff --git a/core/src/cluster_info.rs b/core/src/cluster_info.rs index 81fe75f585..13f1a39704 100644 --- a/core/src/cluster_info.rs +++ b/core/src/cluster_info.rs @@ -683,14 +683,10 @@ impl ClusterInfo { where F: FnOnce(&ContactInfo) -> Y, { - let entry = CrdsValueLabel::ContactInfo(*id); - self.gossip - .read() - .unwrap() - .crds - .lookup(&entry) - .and_then(CrdsValue::contact_info) - .map(map) + let label = CrdsValueLabel::ContactInfo(*id); + let gossip = self.gossip.read().unwrap(); + let entry = gossip.crds.get(&label)?; + Some(map(entry.value.contact_info()?)) } pub fn lookup_contact_info_by_gossip_addr( @@ -715,13 +711,11 @@ impl ClusterInfo { } pub fn lookup_epoch_slots(&self, ix: EpochSlotsIndex) -> EpochSlots { - let entry = CrdsValueLabel::EpochSlots(ix, self.id()); - self.gossip - .read() - .unwrap() - .crds - .lookup(&entry) - .and_then(CrdsValue::epoch_slots) + let label = CrdsValueLabel::EpochSlots(ix, self.id()); + let gossip = self.gossip.read().unwrap(); + let entry = gossip.crds.get(&label); + entry + .and_then(|v| v.value.epoch_slots()) .cloned() .unwrap_or_else(|| EpochSlots::new(self.id(), timestamp())) } @@ -884,8 +878,8 @@ impl ClusterInfo { .read() .unwrap() .crds - .lookup(&CrdsValueLabel::LowestSlot(self.id())) - .and_then(|x| x.lowest_slot()) + .get(&CrdsValueLabel::LowestSlot(self.id())) + .and_then(|x| x.value.lowest_slot()) .map(|x| x.lowest) .unwrap_or(0); if min > last { @@ -910,8 +904,8 @@ impl ClusterInfo { &self.stats.epoch_slots_lookup, ) .crds - .lookup(&CrdsValueLabel::EpochSlots(ix, self.id())) - .and_then(CrdsValue::epoch_slots) + .get(&CrdsValueLabel::EpochSlots(ix, self.id())) + .and_then(|v| v.value.epoch_slots()) .and_then(|x| Some((x.wallclock, x.first_slot()?)))?, ix, )) @@ -1054,9 +1048,9 @@ impl ClusterInfo { (0..MAX_LOCKOUT_HISTORY as u8) .filter_map(|ix| { let vote = CrdsValueLabel::Vote(ix, self_pubkey); - let vote = gossip.crds.lookup(&vote)?; + let vote = gossip.crds.get(&vote)?; num_crds_votes += 1; - match &vote.data { + match &vote.value.data { CrdsData::Vote(_, vote) if should_evict_vote(vote) => { Some((vote.wallclock, ix)) } @@ -1077,8 +1071,8 @@ impl ClusterInfo { self.time_gossip_read_lock("gossip_read_push_vote", &self.stats.push_vote_read); (0..MAX_LOCKOUT_HISTORY as u8).find(|ix| { let vote = CrdsValueLabel::Vote(*ix, self.id()); - if let Some(vote) = gossip.crds.lookup(&vote) { - match &vote.data { + if let Some(vote) = gossip.crds.get(&vote) { + match &vote.value.data { CrdsData::Vote(_, prev_vote) => match prev_vote.slot() { Some(prev_vote_slot) => prev_vote_slot == vote_slot, None => { @@ -3422,13 +3416,8 @@ mod tests { let d = ContactInfo::new_localhost(&solana_sdk::pubkey::new_rand(), timestamp()); let label = CrdsValueLabel::ContactInfo(d.id); cluster_info.insert_info(d); - assert!(cluster_info - .gossip - .read() - .unwrap() - .crds - .lookup(&label) - .is_some()); + let gossip = cluster_info.gossip.read().unwrap(); + assert!(gossip.crds.get(&label).is_some()); } fn assert_in_range(x: u16, range: (u16, u16)) { @@ -3701,7 +3690,7 @@ mod tests { let gossip = cluster_info.gossip.read().unwrap(); let mut vote_slots = HashSet::new(); for label in labels { - match &gossip.crds.lookup(&label).unwrap().data { + match &gossip.crds.get(&label).unwrap().value.data { CrdsData::Vote(_, vote) => { assert!(vote_slots.insert(vote.slot().unwrap())); } diff --git a/core/src/crds.rs b/core/src/crds.rs index 51596671ce..f03848bc18 100644 --- a/core/src/crds.rs +++ b/core/src/crds.rs @@ -222,14 +222,6 @@ impl Crds { } } - pub fn lookup(&self, label: &CrdsValueLabel) -> Option<&CrdsValue> { - self.table.get(label).map(|x| &x.value) - } - - pub fn lookup_versioned(&self, label: &CrdsValueLabel) -> Option<&VersionedCrdsValue> { - self.table.get(label) - } - pub fn get(&self, label: &CrdsValueLabel) -> Option<&VersionedCrdsValue> { self.table.get(label) } diff --git a/core/src/crds_gossip_pull.rs b/core/src/crds_gossip_pull.rs index cc44d1a456..98a6bcd486 100644 --- a/core/src/crds_gossip_pull.rs +++ b/core/src/crds_gossip_pull.rs @@ -1197,14 +1197,8 @@ pub(crate) mod tests { 1, ); assert!(rsp.iter().all(|rsp| rsp.is_empty())); - assert!(dest_crds.lookup(&caller.label()).is_some()); - assert_eq!( - dest_crds - .lookup_versioned(&caller.label()) - .unwrap() - .local_timestamp, - 1 - ); + assert!(dest_crds.get(&caller.label()).is_some()); + assert_eq!(dest_crds.get(&caller.label()).unwrap().local_timestamp, 1); } #[test] fn test_process_pull_request_response() { @@ -1243,13 +1237,7 @@ pub(crate) mod tests { assert_eq!(same_key.label(), new.label()); assert!(same_key.wallclock() < new.wallclock()); node_crds.insert(same_key.clone(), 0).unwrap(); - assert_eq!( - node_crds - .lookup_versioned(&same_key.label()) - .unwrap() - .local_timestamp, - 0 - ); + assert_eq!(node_crds.get(&same_key.label()).unwrap().local_timestamp, 0); let mut done = false; let mut pings = Vec::new(); let ping_cache = Mutex::new(ping_cache); @@ -1300,21 +1288,9 @@ pub(crate) mod tests { ) .0; assert_eq!(failed, 0); - assert_eq!( - node_crds - .lookup_versioned(&new.label()) - .unwrap() - .local_timestamp, - 1 - ); + assert_eq!(node_crds.get(&new.label()).unwrap().local_timestamp, 1); // verify that the whole record was updated for dest since this is a response from dest - assert_eq!( - node_crds - .lookup_versioned(&same_key.label()) - .unwrap() - .local_timestamp, - 1 - ); + assert_eq!(node_crds.get(&same_key.label()).unwrap().local_timestamp, 1); done = true; break; } @@ -1337,19 +1313,23 @@ pub(crate) mod tests { 0, ))); node_crds.insert(old.clone(), 0).unwrap(); - let value_hash = node_crds.lookup_versioned(&old.label()).unwrap().value_hash; + let value_hash = node_crds.get(&old.label()).unwrap().value_hash; //verify self is valid - assert_eq!(node_crds.lookup(&node_label).unwrap().label(), node_label); - + assert_eq!( + node_crds.get(&node_label).unwrap().value.label(), + node_label + ); // purge let timeouts = node.make_timeouts(node_pubkey, &HashMap::new(), Duration::default()); node.purge_active(&thread_pool, &mut node_crds, node.crds_timeout, &timeouts); //verify self is still valid after purge - assert_eq!(node_crds.lookup(&node_label).unwrap().label(), node_label); - - assert_eq!(node_crds.lookup_versioned(&old.label()), None); + assert_eq!( + node_crds.get(&node_label).unwrap().value.label(), + node_label + ); + assert_eq!(node_crds.get(&old.label()), None); assert_eq!(node_crds.num_purged(), 1); for _ in 0..30 { // there is a chance of a false positive with bloom filters diff --git a/core/src/crds_gossip_push.rs b/core/src/crds_gossip_push.rs index 8775ea60a4..595c97680b 100644 --- a/core/src/crds_gossip_push.rs +++ b/core/src/crds_gossip_push.rs @@ -459,7 +459,7 @@ mod test { push.process_push_message(&mut crds, &Pubkey::default(), value.clone(), 0), Ok(()) ); - assert_eq!(crds.lookup(&label), Some(&value)); + assert_eq!(crds.get(&label).unwrap().value, value); // push it again assert_matches!( @@ -843,7 +843,7 @@ mod test { push.process_push_message(&mut crds, &Pubkey::default(), value.clone(), 0), Ok(()) ); - assert_eq!(crds.lookup(&label), Some(&value)); + assert_eq!(crds.get(&label).unwrap().value, value); // push it again assert_matches!( diff --git a/core/tests/crds_gossip.rs b/core/tests/crds_gossip.rs index 732f7eb9bb..04339db235 100644 --- a/core/tests/crds_gossip.rs +++ b/core/tests/crds_gossip.rs @@ -180,13 +180,9 @@ fn ring_network_create(num: usize) -> Network { let start_info = { let start = &network[&keys[k]]; let start_id = start.lock().unwrap().id; - start - .lock() - .unwrap() - .crds - .lookup(&CrdsValueLabel::ContactInfo(start_id)) - .unwrap() - .clone() + let label = CrdsValueLabel::ContactInfo(start_id); + let gossip = start.gossip.lock().unwrap(); + gossip.crds.get(&label).unwrap().value.clone() }; let end = network.get_mut(&keys[(k + 1) % keys.len()]).unwrap(); end.lock() @@ -226,7 +222,7 @@ fn connected_staked_network_create(stakes: &[u64]) -> Network { let start = &network[k].lock().unwrap(); let start_id = start.id; let start_label = CrdsValueLabel::ContactInfo(start_id); - start.crds.lookup(&start_label).unwrap().clone() + start.crds.get(&start_label).unwrap().value.clone() }) .collect(); for end in network.values_mut() { @@ -275,11 +271,9 @@ fn network_simulator(thread_pool: &ThreadPool, network: &mut Network, max_conver // push a message to the network network_values.par_iter().for_each(|locked_node| { let node = &mut locked_node.lock().unwrap(); - let mut m = node - .crds - .lookup(&CrdsValueLabel::ContactInfo(node.id)) - .and_then(|v| v.contact_info().cloned()) - .unwrap(); + let label = CrdsValueLabel::ContactInfo(node.id); + let entry = node.crds.get(&label).unwrap(); + let mut m = entry.value.contact_info().cloned().unwrap(); m.wallclock = now; node.process_push_message( &Pubkey::default(),