From 8c111b06498632d7a3197b7393cabf3bcfc0a501 Mon Sep 17 00:00:00 2001
From: brooks <brooks@anza.xyz>
Date: Tue, 18 Jun 2024 09:51:28 -0400
Subject: [PATCH] Round up correctly when truncating max ancient storages

---
 accounts-db/src/ancient_append_vecs.rs | 54 ++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/accounts-db/src/ancient_append_vecs.rs b/accounts-db/src/ancient_append_vecs.rs
index 392427724a5d4a..9a94be89b60fd0 100644
--- a/accounts-db/src/ancient_append_vecs.rs
+++ b/accounts-db/src/ancient_append_vecs.rs
@@ -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
@@ -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 {
@@ -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 _,
@@ -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(
@@ -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()
@@ -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());
+    }
 }