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

Round up correctly when truncating max ancient storages #1781

Merged
Merged
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
54 changes: 51 additions & 3 deletions accounts-db/src/ancient_append_vecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ impl AncientSlotInfos {
for (i, info) in self.all_infos.iter().enumerate() {
cumulative_bytes += info.alive_bytes;
let ancient_storages_required =
(cumulative_bytes.0 / tuning.ideal_storage_size + 1) as usize;
div_ceil(cumulative_bytes.0, tuning.ideal_storage_size) as usize;
let storages_remaining = total_storages - i - 1;

// if the remaining uncombined storages and the # of resulting
Expand Down Expand Up @@ -1087,6 +1087,25 @@ pub fn is_ancient(storage: &AccountsFile) -> bool {
storage.capacity() >= get_ancient_append_vec_capacity()
}

/// Divides `x` by `y` and rounds up
///
/// # Notes
///
/// It is undefined behavior if `x + y` overflows a u64.
/// Debug builds check this invariant, and will panic if broken.
fn div_ceil(x: u64, y: NonZeroU64) -> u64 {
let y = y.get();
debug_assert!(
x.checked_add(y).is_some(),
"x + y must not overflow! x: {x}, y: {y}",
);
// SAFETY: The caller guaranteed `x + y` does not overflow
// SAFETY: Since `y` is NonZero:
// - we know the denominator is > 0, and thus safe (cannot have divide-by-zero)
// - we know `x + y` is non-zero, and thus the numerator is safe (cannot underflow)
(x + y - 1) / y
}

#[cfg(test)]
pub mod tests {
use {
Expand All @@ -1106,7 +1125,10 @@ pub mod tests {
},
accounts_hash::AccountHash,
accounts_index::UpsertReclaim,
append_vec::{aligned_stored_size, AppendVec, AppendVecStoredAccountMeta},
append_vec::{
aligned_stored_size, AppendVec, AppendVecStoredAccountMeta,
MAXIMUM_APPEND_VEC_FILE_SIZE,
},
storable_accounts::{tests::build_accounts_from_storage, StorableAccountsBySlot},
},
rand::seq::SliceRandom as _,
Expand All @@ -1118,6 +1140,7 @@ pub mod tests {
std::{collections::HashSet, ops::Range},
strum::IntoEnumIterator,
strum_macros::EnumIter,
test_case::test_case,
};

fn get_sample_storages(
Expand Down Expand Up @@ -2853,7 +2876,7 @@ pub mod tests {
.collect();
assert_eq!(
infos.all_infos.len() as u64,
tuning.max_resulting_storages.get() - 1,
tuning.max_resulting_storages.get(),
);
assert!(high_slots
.iter()
Expand Down Expand Up @@ -3786,4 +3809,29 @@ pub mod tests {
assert!(expected_ref_counts.is_empty());
}
}

#[test_case(0, 1 => 0)]
#[test_case(1, 1 => 1)]
#[test_case(2, 1 => 2)]
#[test_case(2, 2 => 1)]
#[test_case(2, 3 => 1)]
#[test_case(2, 4 => 1)]
#[test_case(3, 4 => 1)]
#[test_case(4, 4 => 1)]
#[test_case(5, 4 => 2)]
#[test_case(0, u64::MAX => 0)]
#[test_case(MAXIMUM_APPEND_VEC_FILE_SIZE - 1, MAXIMUM_APPEND_VEC_FILE_SIZE => 1)]
#[test_case(MAXIMUM_APPEND_VEC_FILE_SIZE + 1, MAXIMUM_APPEND_VEC_FILE_SIZE => 2)]
fn test_div_ceil(x: u64, y: u64) -> u64 {
div_ceil(x, NonZeroU64::new(y).unwrap())
}

#[should_panic(expected = "x + y must not overflow")]
#[test_case(1, u64::MAX)]
#[test_case(u64::MAX, 1)]
#[test_case(u64::MAX/2 + 2, u64::MAX/2)]
#[test_case(u64::MAX/2, u64::MAX/2 + 2)]
fn test_div_ceil_overflow(x: u64, y: u64) {
div_ceil(x, NonZeroU64::new(y).unwrap());
}
}