Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stops pushing legacy snapshot hashes to crds #33576

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 3 additions & 15 deletions core/src/snapshot_packager_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mod snapshot_gossip_manager;
use {
crossbeam_channel::{Receiver, Sender},
snapshot_gossip_manager::SnapshotGossipManager,
solana_gossip::cluster_info::{ClusterInfo, MAX_LEGACY_SNAPSHOT_HASHES},
solana_gossip::cluster_info::ClusterInfo,
solana_measure::measure_us,
solana_perf::thread::renice_this_thread,
solana_runtime::{
Expand Down Expand Up @@ -39,25 +39,13 @@ impl SnapshotPackagerService {
snapshot_config: SnapshotConfig,
enable_gossip_push: bool,
) -> Self {
let max_full_snapshot_hashes = std::cmp::min(
MAX_LEGACY_SNAPSHOT_HASHES,
snapshot_config
.maximum_full_snapshot_archives_to_retain
.get(),
);

let t_snapshot_packager = Builder::new()
.name("solSnapshotPkgr".to_string())
.spawn(move || {
info!("SnapshotPackagerService has started");
renice_this_thread(snapshot_config.packager_thread_niceness_adj).unwrap();
let mut snapshot_gossip_manager = enable_gossip_push.then(|| {
SnapshotGossipManager::new(
cluster_info,
max_full_snapshot_hashes,
starting_snapshot_hashes,
)
});
let mut snapshot_gossip_manager = enable_gossip_push
.then(|| SnapshotGossipManager::new(cluster_info, starting_snapshot_hashes));

loop {
if exit.load(Ordering::Relaxed) {
Expand Down
36 changes: 1 addition & 35 deletions core/src/snapshot_packager_service/snapshot_gossip_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use {
snapshot_hash::{
FullSnapshotHash, IncrementalSnapshotHash, SnapshotHash, StartingSnapshotHashes,
},
snapshot_package::{retain_max_n_elements, SnapshotKind},
snapshot_package::SnapshotKind,
},
solana_sdk::{clock::Slot, hash::Hash},
std::sync::Arc,
Expand All @@ -14,8 +14,6 @@ use {
pub struct SnapshotGossipManager {
cluster_info: Arc<ClusterInfo>,
latest_snapshot_hashes: Option<LatestSnapshotHashes>,
max_legacy_full_snapshot_hashes: usize,
legacy_full_snapshot_hashes: Vec<FullSnapshotHash>,
}

impl SnapshotGossipManager {
Expand All @@ -24,14 +22,11 @@ impl SnapshotGossipManager {
#[must_use]
pub fn new(
cluster_info: Arc<ClusterInfo>,
max_legacy_full_snapshot_hashes: usize,
starting_snapshot_hashes: Option<StartingSnapshotHashes>,
) -> Self {
let mut this = SnapshotGossipManager {
cluster_info,
latest_snapshot_hashes: None,
max_legacy_full_snapshot_hashes,
legacy_full_snapshot_hashes: Vec::default(),
};
if let Some(starting_snapshot_hashes) = starting_snapshot_hashes {
this.push_starting_snapshot_hashes(starting_snapshot_hashes);
Expand All @@ -49,10 +44,6 @@ impl SnapshotGossipManager {
);
}
self.push_latest_snapshot_hashes_to_cluster();

// Handle legacy snapshot hashes here too
// Once LegacySnapshotHashes are removed from CRDS, also remove them here
self.push_legacy_full_snapshot_hash(starting_snapshot_hashes.full);
}

/// Push new snapshot hash to the cluster via CRDS
Expand All @@ -78,10 +69,6 @@ impl SnapshotGossipManager {
fn push_full_snapshot_hash(&mut self, full_snapshot_hash: FullSnapshotHash) {
self.update_latest_full_snapshot_hash(full_snapshot_hash);
self.push_latest_snapshot_hashes_to_cluster();

// Handle legacy snapshot hashes here too
// Once LegacySnapshotHashes are removed from CRDS, also remove them here
self.push_legacy_full_snapshot_hash(full_snapshot_hash);
}

/// Push new incremental snapshot hash to the cluster via CRDS
Expand Down Expand Up @@ -146,22 +133,6 @@ impl SnapshotGossipManager {
and a new error case has been added that has not been handled here.",
);
}

/// Add `full_snapshot_hash` to the vector of full snapshot hashes, then push that vector to
/// the cluster via CRDS.
fn push_legacy_full_snapshot_hash(&mut self, full_snapshot_hash: FullSnapshotHash) {
self.legacy_full_snapshot_hashes.push(full_snapshot_hash);

retain_max_n_elements(
&mut self.legacy_full_snapshot_hashes,
self.max_legacy_full_snapshot_hashes,
);

self.cluster_info
.push_legacy_snapshot_hashes(clone_hashes_for_crds(
self.legacy_full_snapshot_hashes.as_slice(),
));
Comment on lines -160 to -163
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main change. Because we remove this call, then all the other code removals fall out from here.

}
}

#[derive(Debug, Copy, Clone, Eq, PartialEq)]
Expand Down Expand Up @@ -191,8 +162,3 @@ impl AsSnapshotHash for IncrementalSnapshotHash {
&self.0
}
}

/// Clones and maps snapshot hashes into what CRDS expects
fn clone_hashes_for_crds(hashes: &[impl AsSnapshotHash]) -> Vec<(Slot, Hash)> {
hashes.iter().map(AsSnapshotHash::clone_for_crds).collect()
}