gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Replace channel with Mutex<Option> for AccountsPackage (#24013)

Browse Source
This commit is contained in:
Brooks Prumo
2022-04-06 05:47:19 -05:00
committed by GitHub
parent 07f4a9040a
commit c322842257
10 changed files with 232 additions and 118 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
runtime/src/accounts_background_service.rs
View File

@ -7,7 +7,7 @@ use {
bank::{Bank, BankSlotDelta, DropCallback},
bank_forks::BankForks,
snapshot_config::SnapshotConfig,
snapshot_package::{AccountsPackageSender, SnapshotType},
snapshot_package::{PendingAccountsPackage, SnapshotType},
snapshot_utils::{self, SnapshotError},
},
crossbeam_channel::{Receiver, SendError, Sender},
@ -85,7 +85,7 @@ pub struct SnapshotRequest {
pub struct SnapshotRequestHandler {
pub snapshot_config: SnapshotConfig,
pub snapshot_request_receiver: SnapshotRequestReceiver,
pub accounts_package_sender: AccountsPackageSender,
pub pending_accounts_package: PendingAccountsPackage,
}
impl SnapshotRequestHandler {
@ -198,7 +198,7 @@ impl SnapshotRequestHandler {
let result = snapshot_utils::snapshot_bank(
&snapshot_root_bank,
status_cache_slot_deltas,
&self.accounts_package_sender,
&self.pending_accounts_package,
&self.snapshot_config.bank_snapshots_dir,
&self.snapshot_config.snapshot_archives_dir,
self.snapshot_config.snapshot_version,
@ -269,7 +269,6 @@ impl SnapshotRequestHandler {
SnapshotError::ArchiveGenerationFailure(..) => true,
SnapshotError::StoragePathSymlinkInvalid => true,
SnapshotError::UnpackError(..) => true,
SnapshotError::AccountsPackageSendError(..) => true,
SnapshotError::IoWithSource(..) => true,
SnapshotError::PathToFileNameError(..) => true,
SnapshotError::FileNameToStrError(..) => true,

12
runtime/src/snapshot_package.rs
View File

@ -10,7 +10,6 @@ use {
TMP_BANK_SNAPSHOT_PREFIX,
},
},
crossbeam_channel::{Receiver, SendError, Sender},
log::*,
solana_sdk::{
clock::Slot, genesis_config::ClusterType, hash::Hash, sysvar::epoch_schedule::EpochSchedule,
@ -23,14 +22,9 @@ use {
tempfile::TempDir,
};
/// The sender side of the AccountsPackage channel, used by AccountsBackgroundService
pub type AccountsPackageSender = Sender<AccountsPackage>;
/// The receiver side of the AccountsPackage channel, used by AccountsHashVerifier
pub type AccountsPackageReceiver = Receiver<AccountsPackage>;
/// The error type when sending an AccountsPackage over the channel fails
pub type AccountsPackageSendError = SendError<AccountsPackage>;
/// The PendingAccountsPackage passes an AccountsPackage from AccountsBackgroundService to
/// AccountsHashVerifier for hashing
pub type PendingAccountsPackage = Arc<Mutex<Option<AccountsPackage>>>;
/// The PendingSnapshotPackage passes a SnapshotPackage from AccountsHashVerifier to
/// SnapshotPackagerService for archiving

137
runtime/src/snapshot_utils.rs
View File

@ -14,8 +14,7 @@ use {
FullSnapshotArchiveInfo, IncrementalSnapshotArchiveInfo, SnapshotArchiveInfoGetter,
},
snapshot_package::{
AccountsPackage, AccountsPackageSendError, AccountsPackageSender, SnapshotPackage,
SnapshotType,
AccountsPackage, PendingAccountsPackage, SnapshotPackage, SnapshotType,
},
},
bincode::{config::Options, serialize_into},
@ -179,9 +178,6 @@ pub enum SnapshotError {
#[error("Unpack error: {0}")]
UnpackError(#[from] UnpackError),
#[error("accounts package send error")]
AccountsPackageSendError(#[from] AccountsPackageSendError),
#[error("source({1}) - I/O error: {0}")]
IoWithSource(std::io::Error, &'static str),
@ -1638,7 +1634,7 @@ where
pub fn snapshot_bank(
root_bank: &Bank,
status_cache_slot_deltas: Vec<BankSlotDelta>,
accounts_package_sender: &AccountsPackageSender,
pending_accounts_package: &PendingAccountsPackage,
bank_snapshots_dir: impl AsRef<Path>,
snapshot_archives_dir: impl AsRef<Path>,
snapshot_version: SnapshotVersion,
@ -1672,7 +1668,23 @@ pub fn snapshot_bank(
)
.expect("failed to hard link bank snapshot into a tmpdir");
accounts_package_sender.send(accounts_package)?;
if can_submit_accounts_package(&accounts_package, pending_accounts_package) {
let old_accounts_package = pending_accounts_package
.lock()
.unwrap()
.replace(accounts_package);
if let Some(old_accounts_package) = old_accounts_package {
debug!(
"The pending AccountsPackage has been overwritten: \
\nNew AccountsPackage slot: {}, snapshot type: {:?} \
\nOld AccountsPackage slot: {}, snapshot type: {:?}",
root_bank.slot(),
snapshot_type,
old_accounts_package.slot,
old_accounts_package.snapshot_type,
);
}
}
Ok(())
}
@ -1877,6 +1889,37 @@ pub fn should_take_incremental_snapshot(
&& last_full_snapshot_slot.is_some()
}
/// Decide if an accounts package can be submitted to the PendingAccountsPackage
///
/// This is based on the values for `snapshot_type` in both the `accounts_package` and the
/// `pending_accounts_package`:
/// - if the new AccountsPackage is for a full snapshot, always submit
/// - if the new AccountsPackage is for an incremental snapshot, submit as long as there isn't a
/// pending full snapshot
/// - otherwise, only submit the new AccountsPackage as long as there's not a pending package
/// destined for a snapshot archive
fn can_submit_accounts_package(
accounts_package: &AccountsPackage,
pending_accounts_package: &PendingAccountsPackage,
) -> bool {
match accounts_package.snapshot_type {
Some(SnapshotType::FullSnapshot) => true,
Some(SnapshotType::IncrementalSnapshot(_)) => pending_accounts_package
.lock()
.unwrap()
.as_ref()
.and_then(|old_accounts_package| old_accounts_package.snapshot_type)
.map(|old_snapshot_type| !old_snapshot_type.is_full_snapshot())
.unwrap_or(true),
None => pending_accounts_package
.lock()
.unwrap()
.as_ref()
.map(|old_accounts_package| old_accounts_package.snapshot_type.is_none())
.unwrap_or(true),
}
}
#[cfg(test)]
mod tests {
use {
@ -3263,4 +3306,84 @@ mod tests {
"Ensure Account1 has not been brought back from the dead"
);
}
/// All the permutations of `snapshot_type` for the new-and-old accounts packages:
///
/// new | old |
/// snapshot | snapshot |
/// type | type | result
/// ----------+----------+--------
/// FSS | FSS | true
/// FSS | ISS | true
/// FSS | None | true
/// ISS | FSS | false
/// ISS | ISS | true
/// ISS | None | true
/// None | FSS | false
/// None | ISS | false
/// None | None | true
#[test]
fn test_can_submit_accounts_package() {
/// helper function to create an AccountsPackage that's good enough for this test
fn new_accounts_package_with(snapshot_type: Option<SnapshotType>) -> AccountsPackage {
AccountsPackage {
slot: Slot::default(),
block_height: Slot::default(),
slot_deltas: Vec::default(),
snapshot_links: TempDir::new().unwrap(),
snapshot_storages: SnapshotStorages::default(),
accounts_hash: Hash::default(),
archive_format: ArchiveFormat::Tar,
snapshot_version: SnapshotVersion::default(),
snapshot_archives_dir: PathBuf::default(),
expected_capitalization: u64::default(),
accounts_hash_for_testing: None,
cluster_type: solana_sdk::genesis_config::ClusterType::Development,
snapshot_type,
accounts: Arc::new(crate::accounts::Accounts::default_for_tests()),
epoch_schedule: solana_sdk::epoch_schedule::EpochSchedule::default(),
rent_collector: crate::rent_collector::RentCollector::default(),
}
}
let pending_accounts_package = PendingAccountsPackage::default();
for (new_snapshot_type, old_snapshot_type, expected_result) in [
(
Some(SnapshotType::FullSnapshot),
Some(SnapshotType::FullSnapshot),
true,
),
(
Some(SnapshotType::FullSnapshot),
Some(SnapshotType::IncrementalSnapshot(0)),
true,
),
(Some(SnapshotType::FullSnapshot), None, true),
(
Some(SnapshotType::IncrementalSnapshot(0)),
Some(SnapshotType::FullSnapshot),
false,
),
(
Some(SnapshotType::IncrementalSnapshot(0)),
Some(SnapshotType::IncrementalSnapshot(0)),
true,
),
(Some(SnapshotType::IncrementalSnapshot(0)), None, true),
(None, Some(SnapshotType::FullSnapshot), false),
(None, Some(SnapshotType::IncrementalSnapshot(0)), false),
(None, None, true),
] {
let new_accounts_package = new_accounts_package_with(new_snapshot_type);
let old_accounts_package = new_accounts_package_with(old_snapshot_type);
pending_accounts_package
.lock()
.unwrap()
.replace(old_accounts_package);
let actual_result =
can_submit_accounts_package(&new_accounts_package, &pending_accounts_package);
assert_eq!(expected_result, actual_result);
}
}
}