Add new error type AccountsFileError (#31631)

#### Problem
AccountsFile- and AppendVec-related code uses io::Error for their main error type.  However, there're many errors under AccountsFile that don't belong to io::Error.

#### Summary of Changes
This PR introduces AccountsFileError and makes minimum changes to keep the PR small.
Subsequent changes related to this will be in separate PRs.
This commit is contained in:
Yueh-Hsuan Chiang 2023-05-22 14:09:09 -07:00 committed by GitHub
parent bea062b1e6
commit c79a6e74e2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 53 additions and 36 deletions

View File

@ -30,7 +30,7 @@ use {
},
accounts_background_service::{DroppedSlotsSender, SendDroppedBankCallback},
accounts_cache::{AccountsCache, CachedAccount, SlotCache},
accounts_file::AccountsFile,
accounts_file::{AccountsFile, AccountsFileError},
accounts_hash::{
AccountsDeltaHash, AccountsHash, AccountsHashEnum, AccountsHasher,
CalcAccountsHashConfig, CalculateHashIntermediate, HashStats, IncrementalAccountsHash,
@ -97,7 +97,7 @@ use {
boxed::Box,
collections::{hash_map, BTreeSet, HashMap, HashSet},
hash::{Hash as StdHash, Hasher as StdHasher},
io::{Error as IoError, Result as IoResult},
io::Result as IoResult,
ops::{Range, RangeBounds},
path::{Path, PathBuf},
str::FromStr,
@ -1138,7 +1138,7 @@ impl AccountStorageEntry {
self.id.load(Ordering::Acquire)
}
pub fn flush(&self) -> Result<(), IoError> {
pub fn flush(&self) -> Result<(), AccountsFileError> {
self.accounts.flush()
}

View File

@ -9,9 +9,10 @@ use {
solana_sdk::{account::ReadableAccount, clock::Slot, hash::Hash, pubkey::Pubkey},
std::{
borrow::Borrow,
io, mem,
mem,
path::{Path, PathBuf},
},
thiserror::Error,
};
// Data placement should be aligned at the next boundary. Without alignment accessing the memory may
@ -24,6 +25,15 @@ macro_rules! u64_align {
};
}
#[derive(Error, Debug)]
/// An enum for AccountsFile related errors.
pub enum AccountsFileError {
#[error("I/O error: {0}")]
Io(#[from] std::io::Error),
}
pub type Result<T> = std::result::Result<T, AccountsFileError>;
#[derive(Debug)]
/// An enum for accessing an accounts file which can be implemented
/// under different formats.
@ -36,7 +46,7 @@ impl AccountsFile {
///
/// The second element of the returned tuple is the number of accounts in the
/// accounts file.
pub fn new_from_file(path: impl AsRef<Path>, current_len: usize) -> io::Result<(Self, usize)> {
pub fn new_from_file(path: impl AsRef<Path>, current_len: usize) -> Result<(Self, usize)> {
let (av, num_accounts) = AppendVec::new_from_file(path, current_len)?;
Ok((Self::AppendVec(av), num_accounts))
}
@ -50,7 +60,7 @@ impl AccountsFile {
}
}
pub fn flush(&self) -> io::Result<()> {
pub fn flush(&self) -> Result<()> {
match self {
Self::AppendVec(av) => av.flush(),
}
@ -109,7 +119,7 @@ impl AccountsFile {
&self,
offset: usize,
owners: &[&Pubkey],
) -> Result<usize, MatchAccountOwnerError> {
) -> std::result::Result<usize, MatchAccountOwnerError> {
match self {
Self::AppendVec(av) => av.account_matches_owners(offset, owners),
}

View File

@ -10,7 +10,7 @@ use {
AccountMeta, StorableAccountsWithHashesAndWriteVersions, StoredAccountInfo,
StoredAccountMeta, StoredMeta, StoredMetaWriteVersion,
},
accounts_file::ALIGN_BOUNDARY_OFFSET,
accounts_file::{AccountsFileError, Result, ALIGN_BOUNDARY_OFFSET},
storable_accounts::StorableAccounts,
u64_align,
},
@ -27,7 +27,7 @@ use {
borrow::Borrow,
convert::TryFrom,
fs::{remove_file, OpenOptions},
io::{self, Seek, SeekFrom, Write},
io::{Seek, SeekFrom, Write},
mem,
path::{Path, PathBuf},
sync::{
@ -295,32 +295,33 @@ impl AppendVec {
self.remove_on_drop = false;
}
fn sanitize_len_and_size(current_len: usize, file_size: usize) -> io::Result<()> {
fn sanitize_len_and_size(current_len: usize, file_size: usize) -> Result<()> {
if file_size == 0 {
Err(std::io::Error::new(
Err(AccountsFileError::Io(std::io::Error::new(
std::io::ErrorKind::Other,
format!("too small file size {file_size} for AppendVec"),
))
)))
} else if usize::try_from(MAXIMUM_APPEND_VEC_FILE_SIZE)
.map(|max| file_size > max)
.unwrap_or(true)
{
Err(std::io::Error::new(
Err(AccountsFileError::Io(std::io::Error::new(
std::io::ErrorKind::Other,
format!("too large file size {file_size} for AppendVec"),
))
)))
} else if current_len > file_size {
Err(std::io::Error::new(
Err(AccountsFileError::Io(std::io::Error::new(
std::io::ErrorKind::Other,
format!("current_len is larger than file size ({file_size})"),
))
)))
} else {
Ok(())
}
}
pub fn flush(&self) -> io::Result<()> {
self.map.flush()
pub fn flush(&self) -> Result<()> {
self.map.flush()?;
Ok(())
}
pub fn reset(&self) {
@ -351,7 +352,7 @@ impl AppendVec {
format!("{slot}.{id}")
}
pub fn new_from_file<P: AsRef<Path>>(path: P, current_len: usize) -> io::Result<(Self, usize)> {
pub fn new_from_file<P: AsRef<Path>>(path: P, current_len: usize) -> Result<(Self, usize)> {
let new = Self::new_from_file_unchecked(&path, current_len)?;
let (sanitized, num_accounts) = new.sanitize_layout_and_length();
@ -362,17 +363,17 @@ impl AppendVec {
"incorrect layout/length/data in the appendvec at path {}",
path.as_ref().display()
);
return Err(std::io::Error::new(std::io::ErrorKind::Other, err_msg));
return Err(AccountsFileError::Io(std::io::Error::new(
std::io::ErrorKind::Other,
err_msg,
)));
}
Ok((new, num_accounts))
}
/// Creates an appendvec from file without performing sanitize checks or counting the number of accounts
pub fn new_from_file_unchecked<P: AsRef<Path>>(
path: P,
current_len: usize,
) -> io::Result<Self> {
pub fn new_from_file_unchecked<P: AsRef<Path>>(path: P, current_len: usize) -> Result<Self> {
let file_size = std::fs::metadata(&path)?.len();
Self::sanitize_len_and_size(current_len, file_size as usize)?;
@ -531,7 +532,7 @@ impl AppendVec {
&self,
offset: usize,
owners: &[&Pubkey],
) -> Result<usize, MatchAccountOwnerError> {
) -> std::result::Result<usize, MatchAccountOwnerError> {
let account_meta = self
.get_account_meta(offset)
.ok_or(MatchAccountOwnerError::UnableToLoad)?;
@ -937,7 +938,7 @@ pub mod tests {
.expect("create a test file for mmap");
let result = AppendVec::new_from_file(path, 0);
assert_matches!(result, Err(ref message) if message.to_string() == *"too small file size 0 for AppendVec");
assert_matches!(result, Err(ref message) if message.to_string().contains("too small file size 0 for AppendVec"));
}
#[test]
@ -945,7 +946,7 @@ pub mod tests {
const LEN: usize = 0;
const SIZE: usize = 0;
let result = AppendVec::sanitize_len_and_size(LEN, SIZE);
assert_matches!(result, Err(ref message) if message.to_string() == *"too small file size 0 for AppendVec");
assert_matches!(result, Err(ref message) if message.to_string().contains("too small file size 0 for AppendVec"));
}
#[test]
@ -961,7 +962,7 @@ pub mod tests {
const LEN: usize = 0;
const SIZE: usize = 16 * 1024 * 1024 * 1024 + 1;
let result = AppendVec::sanitize_len_and_size(LEN, SIZE);
assert_matches!(result, Err(ref message) if message.to_string() == *"too large file size 17179869185 for AppendVec");
assert_matches!(result, Err(ref message) if message.to_string().contains("too large file size 17179869185 for AppendVec"));
}
#[test]
@ -977,7 +978,7 @@ pub mod tests {
const LEN: usize = 1024 * 1024 + 1;
const SIZE: usize = 1024 * 1024;
let result = AppendVec::sanitize_len_and_size(LEN, SIZE);
assert_matches!(result, Err(ref message) if message.to_string() == *"current_len is larger than file size (1048576)");
assert_matches!(result, Err(ref message) if message.to_string().contains("current_len is larger than file size (1048576)"));
}
#[test]
@ -1165,7 +1166,7 @@ pub mod tests {
}
let result = AppendVec::new_from_file(path, accounts_len);
assert_matches!(result, Err(ref message) if message.to_string().starts_with("incorrect layout/length/data"));
assert_matches!(result, Err(ref message) if message.to_string().contains("incorrect layout/length/data"));
}
#[test]
@ -1193,7 +1194,7 @@ pub mod tests {
let accounts_len = av.len();
drop(av);
let result = AppendVec::new_from_file(path, accounts_len);
assert_matches!(result, Err(ref message) if message.to_string().starts_with("incorrect layout/length/data"));
assert_matches!(result, Err(ref message) if message.to_string().contains("incorrect layout/length/data"));
}
#[test]
@ -1219,7 +1220,7 @@ pub mod tests {
let accounts_len = av.len();
drop(av);
let result = AppendVec::new_from_file(path, accounts_len);
assert_matches!(result, Err(ref message) if message.to_string().starts_with("incorrect layout/length/data"));
assert_matches!(result, Err(ref message) if message.to_string().contains("incorrect layout/length/data"));
}
#[test]
@ -1281,7 +1282,7 @@ pub mod tests {
let accounts_len = av.len();
drop(av);
let result = AppendVec::new_from_file(path, accounts_len);
assert_matches!(result, Err(ref message) if message.to_string().starts_with("incorrect layout/length/data"));
assert_matches!(result, Err(ref message) if message.to_string().contains("incorrect layout/length/data"));
}
#[test]

View File

@ -627,7 +627,9 @@ pub(crate) fn reconstruct_single_storage(
current_len: usize,
append_vec_id: AppendVecId,
) -> io::Result<Arc<AccountStorageEntry>> {
let (accounts_file, num_accounts) = AccountsFile::new_from_file(append_vec_path, current_len)?;
let (accounts_file, num_accounts) =
AccountsFile::new_from_file(append_vec_path, current_len)
.map_err(|err| io::Error::new(io::ErrorKind::Other, format!("{}", err)))?;
Ok(Arc::new(AccountStorageEntry::new_existing(
*slot,
append_vec_id,

View File

@ -8,7 +8,7 @@ use {
accounts_db::{
get_temp_accounts_paths, test_utils::create_test_accounts, AccountShrinkThreshold,
},
accounts_file::AccountsFile,
accounts_file::{AccountsFile, AccountsFileError},
accounts_hash::{AccountsDeltaHash, AccountsHash},
bank::{Bank, BankTestConfig},
epoch_accounts_hash,
@ -43,7 +43,7 @@ use {
fn copy_append_vecs<P: AsRef<Path>>(
accounts_db: &AccountsDb,
output_dir: P,
) -> std::io::Result<StorageAndNextAppendVecId> {
) -> Result<StorageAndNextAppendVecId, AccountsFileError> {
let storage_entries = accounts_db.get_snapshot_storages(RangeFull).0;
let storage: AccountStorageMap = AccountStorageMap::with_capacity(storage_entries.len());
let mut next_append_vec_id = 0;

View File

@ -5,6 +5,7 @@ use {
AccountShrinkThreshold, AccountStorageEntry, AccountsDbConfig, AtomicAppendVecId,
CalcAccountsHashDataSource,
},
accounts_file::AccountsFileError,
accounts_hash::AccountsHash,
accounts_index::AccountSecondaryIndexes,
accounts_update_notifier_interface::AccountsUpdateNotifier,
@ -308,6 +309,9 @@ pub enum SnapshotError {
#[error("I/O error: {0}")]
Io(#[from] std::io::Error),
#[error("AccountsFile error: {0}")]
AccountsFileError(#[from] AccountsFileError),
#[error("serialization error: {0}")]
Serialize(#[from] bincode::Error),