From 9d8049df8ee2f050f352f83b63f1d56629d4b60a Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 13 Apr 2023 12:29:03 -0700 Subject: [PATCH 01/13] Add a type_select param for purge_old_bank_snapshots --- core/tests/snapshots.rs | 2 +- local-cluster/tests/local_cluster.rs | 2 +- runtime/src/accounts_background_service.rs | 1 + runtime/src/snapshot_utils.rs | 15 +++++++++++++-- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 17774f2df7309c..3f7334785d2f9d 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -473,7 +473,7 @@ fn test_concurrent_snapshot_packaging( // Purge all the outdated snapshots, including the ones needed to generate the package // currently sitting in the channel - snapshot_utils::purge_old_bank_snapshots(bank_snapshots_dir, MAX_BANK_SNAPSHOTS_TO_RETAIN); + snapshot_utils::purge_old_bank_snapshots(bank_snapshots_dir, MAX_BANK_SNAPSHOTS_TO_RETAIN, None); let mut bank_snapshots = snapshot_utils::get_bank_snapshots_pre(bank_snapshots_dir); bank_snapshots.sort_unstable(); diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index 4da00315fc5215..7d323b1de9638f 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -1102,7 +1102,7 @@ fn test_incremental_snapshot_download_with_crossing_full_snapshot_interval_at_st // To restart, it is not enough to remove the old bank snapshot directories under snapshot/. // The old hardlinks under /snapshot/ should also be removed. // The purge call covers all of them. - snapshot_utils::purge_old_bank_snapshots(validator_snapshot_test_config.bank_snapshots_dir, 0); + snapshot_utils::purge_old_bank_snapshots(validator_snapshot_test_config.bank_snapshots_dir, 0, None); cluster.restart_node( &validator_identity.pubkey(), validator_info, diff --git a/runtime/src/accounts_background_service.rs b/runtime/src/accounts_background_service.rs index dbf23bb7ebd662..e65a98fb45068b 100644 --- a/runtime/src/accounts_background_service.rs +++ b/runtime/src/accounts_background_service.rs @@ -402,6 +402,7 @@ impl SnapshotRequestHandler { snapshot_utils::purge_old_bank_snapshots( &self.snapshot_config.bank_snapshots_dir, MAX_BANK_SNAPSHOTS_TO_RETAIN, + None, ); purge_old_snapshots_time.stop(); total_time.stop(); diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 42eddbd33636c4..5640a832a1f1da 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2936,6 +2936,7 @@ pub fn verify_snapshot_archive( pub fn purge_old_bank_snapshots( bank_snapshots_dir: impl AsRef, num_bank_snapshots_to_retain: usize, + type_select: Option, ) { let do_purge = |mut bank_snapshots: Vec| { bank_snapshots.sort_unstable(); @@ -2954,8 +2955,18 @@ pub fn purge_old_bank_snapshots( }) }; - do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); - do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); + match type_select { + Some(BankSnapshotType::Pre) => { + do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); + } + Some(BankSnapshotType::Post) => { + do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); + } + None => { + do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); + do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); + } + } } /// Get the snapshot storages for this bank From 615bbb45f71829e5e572f00c13730797208b0240 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 13 Apr 2023 12:39:04 -0700 Subject: [PATCH 02/13] Use flags to make the function calls more readable --- core/tests/snapshots.rs | 6 +++++- local-cluster/tests/local_cluster.rs | 6 +++++- runtime/src/snapshot_utils.rs | 14 ++++++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index 3f7334785d2f9d..2a0688c7d8c41b 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -473,7 +473,11 @@ fn test_concurrent_snapshot_packaging( // Purge all the outdated snapshots, including the ones needed to generate the package // currently sitting in the channel - snapshot_utils::purge_old_bank_snapshots(bank_snapshots_dir, MAX_BANK_SNAPSHOTS_TO_RETAIN, None); + snapshot_utils::purge_old_bank_snapshots( + bank_snapshots_dir, + MAX_BANK_SNAPSHOTS_TO_RETAIN, + None, + ); let mut bank_snapshots = snapshot_utils::get_bank_snapshots_pre(bank_snapshots_dir); bank_snapshots.sort_unstable(); diff --git a/local-cluster/tests/local_cluster.rs b/local-cluster/tests/local_cluster.rs index 7d323b1de9638f..b41512f344f994 100644 --- a/local-cluster/tests/local_cluster.rs +++ b/local-cluster/tests/local_cluster.rs @@ -1102,7 +1102,11 @@ fn test_incremental_snapshot_download_with_crossing_full_snapshot_interval_at_st // To restart, it is not enough to remove the old bank snapshot directories under snapshot/. // The old hardlinks under /snapshot/ should also be removed. // The purge call covers all of them. - snapshot_utils::purge_old_bank_snapshots(validator_snapshot_test_config.bank_snapshots_dir, 0, None); + snapshot_utils::purge_old_bank_snapshots( + validator_snapshot_test_config.bank_snapshots_dir, + 0, + None, + ); cluster.restart_node( &validator_identity.pubkey(), validator_info, diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 5640a832a1f1da..9484b5e4eac1e0 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2955,18 +2955,28 @@ pub fn purge_old_bank_snapshots( }) }; + let mut select_pre = false; + let mut select_post = false; match type_select { Some(BankSnapshotType::Pre) => { + select_pre = true; do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); } Some(BankSnapshotType::Post) => { + select_post = true; do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); } None => { - do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); - do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); + select_pre = true; + select_post = true; } } + if select_pre { + do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); + } + if select_post { + do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); + } } /// Get the snapshot storages for this bank From 9ae656bc4146c082c7bf725ae66bdb810a8faa9c Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Thu, 13 Apr 2023 12:40:45 -0700 Subject: [PATCH 03/13] Remove the extra purge calls --- runtime/src/snapshot_utils.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 9484b5e4eac1e0..e77f202624863e 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2960,11 +2960,9 @@ pub fn purge_old_bank_snapshots( match type_select { Some(BankSnapshotType::Pre) => { select_pre = true; - do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); } Some(BankSnapshotType::Post) => { select_post = true; - do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); } None => { select_pre = true; From 037815b808ad7313c3322203f2bc81dd377c933d Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Fri, 14 Apr 2023 14:37:36 -0700 Subject: [PATCH 04/13] replace select_type with filter_by_type --- runtime/src/snapshot_utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index e77f202624863e..e27ecbc35a55af 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2936,7 +2936,7 @@ pub fn verify_snapshot_archive( pub fn purge_old_bank_snapshots( bank_snapshots_dir: impl AsRef, num_bank_snapshots_to_retain: usize, - type_select: Option, + filter_by_type: Option, ) { let do_purge = |mut bank_snapshots: Vec| { bank_snapshots.sort_unstable(); @@ -2957,7 +2957,7 @@ pub fn purge_old_bank_snapshots( let mut select_pre = false; let mut select_post = false; - match type_select { + match filter_by_type { Some(BankSnapshotType::Pre) => { select_pre = true; } From f6fff24810bbe98f629d9f7ad1bc6efb627434bc Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Fri, 14 Apr 2023 18:19:59 -0700 Subject: [PATCH 05/13] Add test --- runtime/src/snapshot_utils.rs | 84 ++++++++++++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index e27ecbc35a55af..ee9e4e22aa370a 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -5522,12 +5522,63 @@ mod tests { assert_eq!(max_id, next_id - 1); } + fn create_snapshot_dirs_for_tests( + genesis_config: &GenesisConfig, + bank_snapshots_dir: impl AsRef, + num_total: usize, + num_posts: usize, + ) -> Bank { + let mut bank = Arc::new(Bank::new_for_tests(&genesis_config)); + + let collecter_id = Pubkey::new_unique(); + let snapshot_version = SnapshotVersion::default(); + + // loop to create the banks at slot 1 to num_total + for _ in 0..num_total { + // prepare the bank + bank = Arc::new(Bank::new_from_parent(&bank, &collecter_id, bank.slot() + 1)); + bank.fill_bank_with_ticks_for_tests(); + bank.squash(); + bank.force_flush_accounts_cache(); + + // generate the bank snapshot directory for slot+1 + let snapshot_storages = bank.get_snapshot_storages(None); + let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas(); + add_bank_snapshot( + &bank_snapshots_dir, + &bank, + &snapshot_storages, + snapshot_version, + slot_deltas, + ) + .unwrap(); + + if bank.slot() as usize > num_posts { + continue; // leave the snapshot dir at PRE stage + } + + // Reserialize the snapshot dir to convert it from PRE to POST, because only the POST type can be used + // to construct a bank. + assert!( + crate::serde_snapshot::reserialize_bank_with_new_accounts_hash( + &bank_snapshots_dir, + bank.slot(), + &AccountsHash(Hash::new_unique()), + None + ) + ); + } + + Arc::try_unwrap(bank).unwrap() + } + #[test] fn test_bank_from_latest_snapshot_dir() { solana_logger::setup(); let genesis_config = GenesisConfig::default(); - let bank_snapshots_dir = tempfile::TempDir::new().unwrap(); + let tmp_dir = tempfile::TempDir::new().unwrap(); + let bank_snapshots_dir = tmp_dir.path(); let bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 3, 3); let account_paths = &bank.rc.accounts.accounts_db.paths; @@ -5554,4 +5605,35 @@ mod tests { "Ensure rebuilding bank from the highest snapshot dir results in the highest bank", ); } + + #[test] + fn test_purge_old_bank_snapshots() { + solana_logger::setup(); + + let genesis_config = GenesisConfig::default(); + let tmp_dir = tempfile::TempDir::new().unwrap(); + let bank_snapshots_dir = tmp_dir.path(); + let bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 10, 5); + // Keep bank in this scope so that its account_paths tmp dirs are not released, and purge_old_bank_snapshots + // can clear the account hardlinks correctly. + let account_paths = &bank.rc.accounts.accounts_db.paths; + info!("account_paths: {:?}", account_paths); + + assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 10); + + purge_old_bank_snapshots(&bank_snapshots_dir, 3, Some(BankSnapshotType::Pre)); + assert_eq!(get_bank_snapshots_pre(&bank_snapshots_dir).len(), 3); + + purge_old_bank_snapshots(&bank_snapshots_dir, 2, Some(BankSnapshotType::Post)); + assert_eq!(get_bank_snapshots_post(&bank_snapshots_dir).len(), 2); + + assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 5); + + purge_old_bank_snapshots(&bank_snapshots_dir, 2, None); + // In the current implementation num_bank_snapshots_to_retain is really per type, so 2 PREs and 2 POSTs are retained + assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 4); + + purge_old_bank_snapshots(&bank_snapshots_dir, 0, None); + assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 0); + } } From 71c0ba3e31712236bc853c8eed0ada531e1255d6 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Fri, 14 Apr 2023 19:40:20 -0700 Subject: [PATCH 06/13] Use matches --- runtime/src/snapshot_utils.rs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index ee9e4e22aa370a..67db39cd571d14 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2955,24 +2955,10 @@ pub fn purge_old_bank_snapshots( }) }; - let mut select_pre = false; - let mut select_post = false; - match filter_by_type { - Some(BankSnapshotType::Pre) => { - select_pre = true; - } - Some(BankSnapshotType::Post) => { - select_post = true; - } - None => { - select_pre = true; - select_post = true; - } - } - if select_pre { + if matches!(filter_by_type, Some(BankSnapshotType::Pre) | None) { do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); } - if select_post { + if matches!(filter_by_type, Some(BankSnapshotType::Post) | None) { do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); } } From 90f2069bba302214648fcdf7410feed1a6026493 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Fri, 14 Apr 2023 20:20:17 -0700 Subject: [PATCH 07/13] Fix CI test on reference --- runtime/src/snapshot_utils.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 67db39cd571d14..15ec58197ddec5 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -5514,7 +5514,7 @@ mod tests { num_total: usize, num_posts: usize, ) -> Bank { - let mut bank = Arc::new(Bank::new_for_tests(&genesis_config)); + let mut bank = Arc::new(Bank::new_for_tests(genesis_config)); let collecter_id = Pubkey::new_unique(); let snapshot_version = SnapshotVersion::default(); @@ -5565,7 +5565,7 @@ mod tests { let genesis_config = GenesisConfig::default(); let tmp_dir = tempfile::TempDir::new().unwrap(); let bank_snapshots_dir = tmp_dir.path(); - let bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 3, 3); + let bank = create_snapshot_dirs_for_tests(&genesis_config, bank_snapshots_dir, 3, 3); let account_paths = &bank.rc.accounts.accounts_db.paths; @@ -5599,27 +5599,27 @@ mod tests { let genesis_config = GenesisConfig::default(); let tmp_dir = tempfile::TempDir::new().unwrap(); let bank_snapshots_dir = tmp_dir.path(); - let bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 10, 5); + let bank = create_snapshot_dirs_for_tests(&genesis_config, bank_snapshots_dir, 10, 5); // Keep bank in this scope so that its account_paths tmp dirs are not released, and purge_old_bank_snapshots // can clear the account hardlinks correctly. let account_paths = &bank.rc.accounts.accounts_db.paths; info!("account_paths: {:?}", account_paths); - assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 10); + assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 10); - purge_old_bank_snapshots(&bank_snapshots_dir, 3, Some(BankSnapshotType::Pre)); - assert_eq!(get_bank_snapshots_pre(&bank_snapshots_dir).len(), 3); + purge_old_bank_snapshots(bank_snapshots_dir, 3, Some(BankSnapshotType::Pre)); + assert_eq!(get_bank_snapshots_pre(bank_snapshots_dir).len(), 3); - purge_old_bank_snapshots(&bank_snapshots_dir, 2, Some(BankSnapshotType::Post)); - assert_eq!(get_bank_snapshots_post(&bank_snapshots_dir).len(), 2); + purge_old_bank_snapshots(bank_snapshots_dir, 2, Some(BankSnapshotType::Post)); + assert_eq!(get_bank_snapshots_post(bank_snapshots_dir).len(), 2); - assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 5); + assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 5); - purge_old_bank_snapshots(&bank_snapshots_dir, 2, None); + purge_old_bank_snapshots(bank_snapshots_dir, 2, None); // In the current implementation num_bank_snapshots_to_retain is really per type, so 2 PREs and 2 POSTs are retained - assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 4); + assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 4); - purge_old_bank_snapshots(&bank_snapshots_dir, 0, None); - assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 0); + purge_old_bank_snapshots(bank_snapshots_dir, 0, None); + assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 0); } } From 494fc7180794d911b0f11a0499edeb424b1ae07d Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Sat, 15 Apr 2023 20:48:51 -0700 Subject: [PATCH 08/13] use match and call do_purge once --- runtime/src/snapshot_utils.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 15ec58197ddec5..b70e7b3eb8c435 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -2955,12 +2955,12 @@ pub fn purge_old_bank_snapshots( }) }; - if matches!(filter_by_type, Some(BankSnapshotType::Pre) | None) { - do_purge(get_bank_snapshots_pre(&bank_snapshots_dir)); - } - if matches!(filter_by_type, Some(BankSnapshotType::Post) | None) { - do_purge(get_bank_snapshots_post(&bank_snapshots_dir)); - } + let bank_snapshots = match filter_by_type { + Some(BankSnapshotType::Pre) => get_bank_snapshots_pre(&bank_snapshots_dir), + Some(BankSnapshotType::Post) => get_bank_snapshots_post(&bank_snapshots_dir), + None => get_bank_snapshots(&bank_snapshots_dir), + }; + do_purge(bank_snapshots); } /// Get the snapshot storages for this bank @@ -5616,8 +5616,7 @@ mod tests { assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 5); purge_old_bank_snapshots(bank_snapshots_dir, 2, None); - // In the current implementation num_bank_snapshots_to_retain is really per type, so 2 PREs and 2 POSTs are retained - assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 4); + assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 2); purge_old_bank_snapshots(bank_snapshots_dir, 0, None); assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 0); From 60e936fc3b693cd7e1f38cec80e687fef3f0b481 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Sat, 15 Apr 2023 21:02:49 -0700 Subject: [PATCH 09/13] Let bank_snapshots_dir be TempDir --- runtime/src/snapshot_utils.rs | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index b70e7b3eb8c435..193eee268434dd 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -5128,8 +5128,6 @@ mod tests { num_total: usize, num_posts: usize, ) -> Bank { - let mut bank = Arc::new(Bank::new_for_tests(genesis_config)); - let collecter_id = Pubkey::new_unique(); let snapshot_version = SnapshotVersion::default(); @@ -5179,6 +5177,8 @@ mod tests { let bank_snapshots_dir = tempfile::TempDir::new().unwrap(); let _bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 4, 0); +======= +>>>>>>> f9b76e846c (Let bank_snapshots_dir be TempDir) let snapshot = get_highest_bank_snapshot(&bank_snapshots_dir).unwrap(); assert_eq!(snapshot.slot, 4); @@ -5563,9 +5563,8 @@ mod tests { solana_logger::setup(); let genesis_config = GenesisConfig::default(); - let tmp_dir = tempfile::TempDir::new().unwrap(); - let bank_snapshots_dir = tmp_dir.path(); - let bank = create_snapshot_dirs_for_tests(&genesis_config, bank_snapshots_dir, 3, 3); + let bank_snapshots_dir = tempfile::TempDir::new().unwrap(); + let bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 3, 3); let account_paths = &bank.rc.accounts.accounts_db.paths; @@ -5597,28 +5596,27 @@ mod tests { solana_logger::setup(); let genesis_config = GenesisConfig::default(); - let tmp_dir = tempfile::TempDir::new().unwrap(); - let bank_snapshots_dir = tmp_dir.path(); - let bank = create_snapshot_dirs_for_tests(&genesis_config, bank_snapshots_dir, 10, 5); + let bank_snapshots_dir = tempfile::TempDir::new().unwrap(); + let bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 10, 5); // Keep bank in this scope so that its account_paths tmp dirs are not released, and purge_old_bank_snapshots // can clear the account hardlinks correctly. let account_paths = &bank.rc.accounts.accounts_db.paths; info!("account_paths: {:?}", account_paths); - assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 10); + assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 10); - purge_old_bank_snapshots(bank_snapshots_dir, 3, Some(BankSnapshotType::Pre)); - assert_eq!(get_bank_snapshots_pre(bank_snapshots_dir).len(), 3); + purge_old_bank_snapshots(&bank_snapshots_dir, 3, Some(BankSnapshotType::Pre)); + assert_eq!(get_bank_snapshots_pre(&bank_snapshots_dir).len(), 3); - purge_old_bank_snapshots(bank_snapshots_dir, 2, Some(BankSnapshotType::Post)); - assert_eq!(get_bank_snapshots_post(bank_snapshots_dir).len(), 2); + purge_old_bank_snapshots(&bank_snapshots_dir, 2, Some(BankSnapshotType::Post)); + assert_eq!(get_bank_snapshots_post(&bank_snapshots_dir).len(), 2); - assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 5); + assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 5); - purge_old_bank_snapshots(bank_snapshots_dir, 2, None); - assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 2); + purge_old_bank_snapshots(&bank_snapshots_dir, 2, None); + assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 2); - purge_old_bank_snapshots(bank_snapshots_dir, 0, None); - assert_eq!(get_bank_snapshots(bank_snapshots_dir).len(), 0); + purge_old_bank_snapshots(&bank_snapshots_dir, 0, None); + assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 0); } } From ed7cd9ad561c6d238b59d6955c5e44a671bb06b1 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Sat, 15 Apr 2023 21:04:17 -0700 Subject: [PATCH 10/13] remove account_paths in the test --- runtime/src/snapshot_utils.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 193eee268434dd..06c6b73e1406fd 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -5600,8 +5600,6 @@ mod tests { let bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 10, 5); // Keep bank in this scope so that its account_paths tmp dirs are not released, and purge_old_bank_snapshots // can clear the account hardlinks correctly. - let account_paths = &bank.rc.accounts.accounts_db.paths; - info!("account_paths: {:?}", account_paths); assert_eq!(get_bank_snapshots(&bank_snapshots_dir).len(), 10); From 19a21b794362b2c5266a653e2f459b9c4af842b3 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Sat, 15 Apr 2023 21:38:13 -0700 Subject: [PATCH 11/13] replace bank with _bank --- runtime/src/snapshot_utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 06c6b73e1406fd..3bac5ad1972be1 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -5597,7 +5597,7 @@ mod tests { let genesis_config = GenesisConfig::default(); let bank_snapshots_dir = tempfile::TempDir::new().unwrap(); - let bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 10, 5); + let _bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 10, 5); // Keep bank in this scope so that its account_paths tmp dirs are not released, and purge_old_bank_snapshots // can clear the account hardlinks correctly. From 877553afe5ba53bff714c6d632939b5c837ff132 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Mon, 17 Apr 2023 14:17:19 -0700 Subject: [PATCH 12/13] Remove create_snapshot_dirs_for_tests, will take the lastest from master --- runtime/src/snapshot_utils.rs | 50 ----------------------------------- 1 file changed, 50 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index 3bac5ad1972be1..d5918290ef18ac 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -5508,56 +5508,6 @@ mod tests { assert_eq!(max_id, next_id - 1); } - fn create_snapshot_dirs_for_tests( - genesis_config: &GenesisConfig, - bank_snapshots_dir: impl AsRef, - num_total: usize, - num_posts: usize, - ) -> Bank { - let mut bank = Arc::new(Bank::new_for_tests(genesis_config)); - - let collecter_id = Pubkey::new_unique(); - let snapshot_version = SnapshotVersion::default(); - - // loop to create the banks at slot 1 to num_total - for _ in 0..num_total { - // prepare the bank - bank = Arc::new(Bank::new_from_parent(&bank, &collecter_id, bank.slot() + 1)); - bank.fill_bank_with_ticks_for_tests(); - bank.squash(); - bank.force_flush_accounts_cache(); - - // generate the bank snapshot directory for slot+1 - let snapshot_storages = bank.get_snapshot_storages(None); - let slot_deltas = bank.status_cache.read().unwrap().root_slot_deltas(); - add_bank_snapshot( - &bank_snapshots_dir, - &bank, - &snapshot_storages, - snapshot_version, - slot_deltas, - ) - .unwrap(); - - if bank.slot() as usize > num_posts { - continue; // leave the snapshot dir at PRE stage - } - - // Reserialize the snapshot dir to convert it from PRE to POST, because only the POST type can be used - // to construct a bank. - assert!( - crate::serde_snapshot::reserialize_bank_with_new_accounts_hash( - &bank_snapshots_dir, - bank.slot(), - &AccountsHash(Hash::new_unique()), - None - ) - ); - } - - Arc::try_unwrap(bank).unwrap() - } - #[test] fn test_bank_from_latest_snapshot_dir() { solana_logger::setup(); From 7b436174b797eec8249331a3054858c19fd6b8d5 Mon Sep 17 00:00:00 2001 From: Xiang Zhu Date: Mon, 17 Apr 2023 14:28:50 -0700 Subject: [PATCH 13/13] Fix merge errors --- runtime/src/snapshot_utils.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/src/snapshot_utils.rs b/runtime/src/snapshot_utils.rs index d5918290ef18ac..78350bff301675 100644 --- a/runtime/src/snapshot_utils.rs +++ b/runtime/src/snapshot_utils.rs @@ -5128,6 +5128,8 @@ mod tests { num_total: usize, num_posts: usize, ) -> Bank { + let mut bank = Arc::new(Bank::new_for_tests(genesis_config)); + let collecter_id = Pubkey::new_unique(); let snapshot_version = SnapshotVersion::default(); @@ -5177,8 +5179,6 @@ mod tests { let bank_snapshots_dir = tempfile::TempDir::new().unwrap(); let _bank = create_snapshot_dirs_for_tests(&genesis_config, &bank_snapshots_dir, 4, 0); -======= ->>>>>>> f9b76e846c (Let bank_snapshots_dir be TempDir) let snapshot = get_highest_bank_snapshot(&bank_snapshots_dir).unwrap(); assert_eq!(snapshot.slot, 4);