adds the instance token to crds-labels for node-instance crds-values (bp #14037) (#14039)

* adds the instance token to crds-labels for node-instance crds-values (#14037)

If a node "a" receives instance-info from node "b1" it will override any
instance-info associated with "b1" pubkey in its crds table. This makes
it less likely that when "b1" receives crds values from "a" (either
through pull or push), it sees other instances of itself (because node
"a" discarded them when it received "b1" instance info).

In order for the crds table to contain all instance-info associated with
the same pubkey at the same time, we need to add the instance tokens to
the keys in the crds table (i.e. the CrdsValueLabel).

(cherry picked from commit 409fe3bca1)

# Conflicts:
#	core/src/cluster_info.rs
#	core/src/crds_value.rs

* removes backport merge conflicts

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
This commit is contained in:
mergify[bot]
2020-12-10 18:25:52 +00:00
committed by GitHub
parent fa70b0f2ff
commit 6812828ba8
2 changed files with 67 additions and 15 deletions

View File

@ -494,7 +494,7 @@ impl ClusterInfo {
id,
stats: GossipStats::default(),
socket: UdpSocket::bind("0.0.0.0:0").unwrap(),
instance: NodeInstance::new(id, timestamp()),
instance: NodeInstance::new(&mut rand::thread_rng(), id, timestamp()),
};
{
let mut gossip = me.gossip.write().unwrap();
@ -522,7 +522,7 @@ impl ClusterInfo {
id: *new_id,
stats: GossipStats::default(),
socket: UdpSocket::bind("0.0.0.0:0").unwrap(),
instance: NodeInstance::new(*new_id, timestamp()),
instance: NodeInstance::new(&mut rand::thread_rng(), *new_id, timestamp()),
}
}

View File

@ -2,7 +2,7 @@ use crate::contact_info::ContactInfo;
use crate::deprecated;
use crate::epoch_slots::EpochSlots;
use bincode::{serialize, serialized_size};
use rand::Rng;
use rand::{CryptoRng, Rng};
use solana_sdk::sanitize::{Sanitize, SanitizeError};
use solana_sdk::timing::timestamp;
use solana_sdk::{
@ -266,12 +266,15 @@ pub struct NodeInstance {
}
impl NodeInstance {
pub fn new(pubkey: Pubkey, now: u64) -> Self {
pub fn new<R>(rng: &mut R, pubkey: Pubkey, now: u64) -> Self
where
R: Rng + CryptoRng,
{
Self {
from: pubkey,
wallclock: now,
timestamp: now,
token: rand::thread_rng().gen(),
token: rng.gen(),
}
}
@ -318,7 +321,7 @@ pub enum CrdsValueLabel {
AccountsHashes(Pubkey),
LegacyVersion(Pubkey),
Version(Pubkey),
NodeInstance(Pubkey),
NodeInstance(Pubkey, u64 /*token*/),
}
impl fmt::Display for CrdsValueLabel {
@ -332,7 +335,7 @@ impl fmt::Display for CrdsValueLabel {
CrdsValueLabel::AccountsHashes(_) => write!(f, "AccountsHashes({})", self.pubkey()),
CrdsValueLabel::LegacyVersion(_) => write!(f, "LegacyVersion({})", self.pubkey()),
CrdsValueLabel::Version(_) => write!(f, "Version({})", self.pubkey()),
CrdsValueLabel::NodeInstance(_) => write!(f, "NodeInstance({})", self.pubkey()),
CrdsValueLabel::NodeInstance(_, _) => write!(f, "NodeInstance({})", self.pubkey()),
}
}
}
@ -348,7 +351,7 @@ impl CrdsValueLabel {
CrdsValueLabel::AccountsHashes(p) => *p,
CrdsValueLabel::LegacyVersion(p) => *p,
CrdsValueLabel::Version(p) => *p,
CrdsValueLabel::NodeInstance(p) => *p,
CrdsValueLabel::NodeInstance(p, _ /*token*/) => *p,
}
}
}
@ -416,7 +419,7 @@ impl CrdsValue {
CrdsData::EpochSlots(ix, _) => CrdsValueLabel::EpochSlots(*ix, self.pubkey()),
CrdsData::LegacyVersion(_) => CrdsValueLabel::LegacyVersion(self.pubkey()),
CrdsData::Version(_) => CrdsValueLabel::Version(self.pubkey()),
CrdsData::NodeInstance(_) => CrdsValueLabel::NodeInstance(self.pubkey()),
CrdsData::NodeInstance(node) => CrdsValueLabel::NodeInstance(self.pubkey(), node.token),
}
}
pub fn contact_info(&self) -> Option<&ContactInfo> {
@ -482,6 +485,8 @@ impl CrdsValue {
}
/// Return all the possible labels for a record identified by Pubkey.
/// Excludes NodeInstance, which is pushed priodically, and does not need
/// to update its local-timestmap.
pub fn record_labels(key: &Pubkey) -> Vec<CrdsValueLabel> {
let mut labels = vec![
CrdsValueLabel::ContactInfo(*key),
@ -490,7 +495,6 @@ impl CrdsValue {
CrdsValueLabel::AccountsHashes(*key),
CrdsValueLabel::LegacyVersion(*key),
CrdsValueLabel::Version(*key),
CrdsValueLabel::NodeInstance(*key),
];
labels.extend((0..MAX_VOTES).map(|ix| CrdsValueLabel::Vote(ix, *key)));
labels.extend((0..MAX_EPOCH_SLOTS).map(|ix| CrdsValueLabel::EpochSlots(ix, *key)));
@ -550,7 +554,7 @@ mod test {
#[test]
fn test_labels() {
let mut hits = [false; 7 + MAX_VOTES as usize + MAX_EPOCH_SLOTS as usize];
let mut hits = [false; 6 + MAX_VOTES as usize + MAX_EPOCH_SLOTS as usize];
// this method should cover all the possible labels
for v in &CrdsValue::record_labels(&Pubkey::default()) {
match v {
@ -560,10 +564,10 @@ mod test {
CrdsValueLabel::AccountsHashes(_) => hits[3] = true,
CrdsValueLabel::LegacyVersion(_) => hits[4] = true,
CrdsValueLabel::Version(_) => hits[5] = true,
CrdsValueLabel::NodeInstance(_) => hits[6] = true,
CrdsValueLabel::Vote(ix, _) => hits[*ix as usize + 7] = true,
CrdsValueLabel::NodeInstance(_, _) => panic!("NodeInstance!?"),
CrdsValueLabel::Vote(ix, _) => hits[*ix as usize + 6] = true,
CrdsValueLabel::EpochSlots(ix, _) => {
hits[*ix as usize + MAX_VOTES as usize + 7] = true
hits[*ix as usize + MAX_VOTES as usize + 6] = true
}
}
}
@ -739,6 +743,53 @@ mod test {
serialize_deserialize_value(value, correct_keypair);
}
#[test]
fn test_node_instance_crds_lable() {
fn make_crds_value(node: NodeInstance) -> CrdsValue {
CrdsValue::new_unsigned(CrdsData::NodeInstance(node))
}
let mut rng = rand::thread_rng();
let now = timestamp();
let pubkey = Pubkey::new_unique();
let node = NodeInstance::new(&mut rng, pubkey, now);
assert_eq!(
make_crds_value(node.clone()).label(),
make_crds_value(node.with_wallclock(now + 8)).label()
);
let other = NodeInstance {
from: Pubkey::new_unique(),
..node
};
assert_ne!(
make_crds_value(node.clone()).label(),
make_crds_value(other).label()
);
let other = NodeInstance {
wallclock: now + 8,
..node
};
assert_eq!(
make_crds_value(node.clone()).label(),
make_crds_value(other).label()
);
let other = NodeInstance {
timestamp: now + 8,
..node
};
assert_eq!(
make_crds_value(node.clone()).label(),
make_crds_value(other).label()
);
let other = NodeInstance {
token: rng.gen(),
..node
};
assert_ne!(
make_crds_value(node).label(),
make_crds_value(other).label()
);
}
#[test]
fn test_check_duplicate_instance() {
fn make_crds_value(node: NodeInstance) -> CrdsValue {
@ -747,7 +798,7 @@ mod test {
let now = timestamp();
let mut rng = rand::thread_rng();
let pubkey = Pubkey::new_unique();
let node = NodeInstance::new(pubkey, now);
let node = NodeInstance::new(&mut rng, pubkey, now);
// Same token is not a duplicate.
assert!(!node.check_duplicate(&make_crds_value(NodeInstance {
from: pubkey,
@ -808,6 +859,7 @@ mod test {
.should_force_push(&pubkey)
);
let node = CrdsValue::new_unsigned(CrdsData::NodeInstance(NodeInstance::new(
&mut rng,
pubkey,
timestamp(),
)));