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

Commit

Permalink
uses current local timestamp when recording purged values
Browse files Browse the repository at this point in the history
CrdsGossipPull.purged_values is meant to record recently purged values
so that they are excluded from imminent pull requests, until the entire
cluster have synced to the updated value:
https://github.com/solana-labs/solana/blob/c826cddbb/core/src/crds_gossip_pull.rs#L449-L454

However, VersionedCrdsValue.local_timestamp represents the local time
when the value was last updated, and given that crds values may have
different timeouts based on stake, it does not necessarily represent how
recently the value was purged:
https://github.com/solana-labs/solana/blob/c826cddbb/core/src/crds.rs#L75-L76

As such, recording current local timestamp when purging values is more
appropriate. Additionally, purge_purged assumes that the purge_values is
sorted in timestamps when draining the old ones; which is not true if
those timestamps are VersionedCrdsValue.local_timestamp:
https://github.com/solana-labs/solana/blob/c826cddbb/core/src/crds_gossip_pull.rs#L563-L571
  • Loading branch information
behzadnouri committed Apr 15, 2021
1 parent c826cdd commit a213355
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 14 deletions.
10 changes: 5 additions & 5 deletions core/src/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1893,11 +1893,11 @@ impl ClusterInfo {
self.stats
.trim_crds_table_purged_values_count
.add_relaxed(purged_values.len() as u64);
gossip.pull.purged_values.extend(
purged_values
.into_iter()
.map(|v| (v.value_hash, v.local_timestamp)),
);
let now = timestamp();
gossip
.pull
.purged_values
.extend(purged_values.into_iter().map(|v| (v.value_hash, now)));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/crds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ pub struct VersionedCrdsValue {
/// local time when inserted
pub insert_timestamp: u64,
/// local time when updated
pub local_timestamp: u64,
pub(crate) local_timestamp: u64,
/// value hash
pub value_hash: Hash,
}
Expand Down
3 changes: 1 addition & 2 deletions core/src/crds_gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,7 @@ impl CrdsGossip {
.push
.process_push_message(&mut self.crds, from, val, now);
if let Ok(Some(val)) = res {
self.pull
.record_old_hash(val.value_hash, val.local_timestamp);
self.pull.record_old_hash(val.value_hash, now);
Some(val)
} else {
None
Expand Down
10 changes: 4 additions & 6 deletions core/src/crds_gossip_pull.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,7 @@ impl CrdsGossipPull {
for caller in callers {
let key = caller.label().pubkey();
if let Ok(Some(val)) = crds.insert(caller, now) {
self.purged_values
.push_back((val.value_hash, val.local_timestamp));
self.purged_values.push_back((val.value_hash, now));
}
crds.update_record_timestamp(&key, now);
}
Expand Down Expand Up @@ -399,8 +398,7 @@ impl CrdsGossipPull {
owners.insert(label.pubkey());
success.push((label, hash, wc));
if let Some(val) = old {
self.purged_values
.push_back((val.value_hash, val.local_timestamp))
self.purged_values.push_back((val.value_hash, now))
}
}
}
Expand Down Expand Up @@ -555,7 +553,7 @@ impl CrdsGossipPull {
.into_iter()
.filter_map(|label| {
let val = crds.remove(&label)?;
Some((val.value_hash, val.local_timestamp))
Some((val.value_hash, now))
}),
);
self.purged_values.len() - num_purged_values
Expand Down Expand Up @@ -1335,7 +1333,7 @@ mod test {
}

// purge the value
node.purge_purged(1);
node.purge_purged(3);
assert_eq!(node.purged_values.len(), 0);
}
#[test]
Expand Down

0 comments on commit a213355

Please sign in to comment.