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

ancient pack: add low water mark #33785

Merged
merged 1 commit into from
Oct 23, 2023
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
72 changes: 48 additions & 24 deletions accounts-db/src/ancient_append_vecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,15 +182,17 @@ impl AncientSlotInfos {
self.shrink_indexes.clear();
let total_storages = self.all_infos.len();
let mut cumulative_bytes = 0u64;
let low_threshold = max_storages * 50 / 100;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the current impl preferred over a divide by two?

Suggested change
let low_threshold = max_storages * 50 / 100;
let low_threshold = max_storages / 2;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps, introduce a const variable for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking this is more tunable than /2. 80%, 20%, etc. Ultimately we might make this a tuning parameter? Even a cli argument? This choice seems sufficient for the moment based on mnb monitoring. And it beats the current 100 slots at a time impl.

for (i, info) in self.all_infos.iter().enumerate() {
saturating_add_assign!(cumulative_bytes, info.alive_bytes);
let ancient_storages_required = (cumulative_bytes / ideal_storage_size + 1) as usize;
let storages_remaining = total_storages - i - 1;

// if the remaining uncombined storages and the # of resulting
// combined ancient storages is less than the threshold, then
// we've gone too far, so get rid of this entry and all after it.
// Every storage after this one is larger.
if storages_remaining + ancient_storages_required < max_storages {
if storages_remaining + ancient_storages_required < low_threshold {
self.all_infos.truncate(i);
break;
}
Expand Down Expand Up @@ -2377,25 +2379,28 @@ pub mod tests {

#[test]
fn test_filter_by_smaller_capacity_sort() {
// max is 3
// 4 storages
// storage[3] is big enough to cause us to need another storage
// so, storage[0] and [1] can be combined into 1, resulting in 3 remaining storages, which is
// the goal, so we only have to combine the first 2 to hit the goal
// max is 6
// 7 storages
// storage[last] is big enough to cause us to need another storage
// so, storage[0..=4] can be combined into 1, resulting in 3 remaining storages, which is
// the goal, so we only have to combine the first 5 to hit the goal
for method in TestSmallestCapacity::iter() {
let ideal_storage_size_large = get_ancient_append_vec_capacity();
for reorder in [false, true] {
let mut infos = create_test_infos(4);
let mut infos = create_test_infos(7);
infos
.all_infos
.iter_mut()
.enumerate()
.for_each(|(i, info)| info.capacity = 1 + i as u64);
if reorder {
infos.all_infos[3].capacity = 0; // sort to beginning
infos.all_infos.last_mut().unwrap().capacity = 0; // sort to beginning
}
infos.all_infos[3].alive_bytes = ideal_storage_size_large;
let max_storages = 3;
infos.all_infos.last_mut().unwrap().alive_bytes = ideal_storage_size_large;
// if we use max_storages = 3 or 4, then the low limit is 1 or 2. To get below 2 requires a result of 1, which packs everyone.
// This isn't what we want for this test. So, we make max_storages big enough that we can get to something reasonable (like 3)
// for a low mark.
let max_storages = 6;
match method {
TestSmallestCapacity::FilterBySmallestCapacity => {
infos.filter_by_smallest_capacity(
Expand All @@ -2421,8 +2426,12 @@ pub mod tests {
.iter()
.map(|info| info.slot)
.collect::<Vec<_>>(),
if reorder { vec![3, 0, 1] } else { vec![0, 1] },
"reorder: {reorder}"
if reorder {
vec![6, 0, 1, 2, 3, 4]
} else {
vec![0, 1, 2, 3, 4]
},
"reorder: {reorder}, method: {method:?}"
);
}
}
Expand Down Expand Up @@ -2468,7 +2477,20 @@ pub mod tests {
max_storages,
NonZeroU64::new(ideal_storage_size_large).unwrap(),
);
assert!(infos.all_infos.is_empty());

if filter {
assert!(infos.all_infos.is_empty());
} else {
// no short circuit, so truncate to shrink below low water
assert_eq!(
infos
.all_infos
.iter()
.map(|info| info.slot)
.collect::<Vec<_>>(),
vec![0]
);
}

let mut infos = create_test_infos(1);
infos.all_infos[0].alive_bytes = ideal_storage_size_large + 1;
Expand Down Expand Up @@ -2501,14 +2523,14 @@ pub mod tests {
assert_eq!(infos.all_infos.len(), 2);
}

// max is 3
// 4 storages
// storage[3] is big enough to cause us to need another storage
// so, storage[0] and [1] can be combined into 1, resulting in 3 remaining storages, which is
// the goal, so we only have to combine the first 2 to hit the goal
let mut infos = create_test_infos(4);
infos.all_infos[3].alive_bytes = ideal_storage_size_large;
let max_storages = 3;
// max is 4
// 5 storages
// storage[4] is big enough to cause us to need another storage
// so, storage[0..=2] can be combined into 1, resulting in 3 remaining storages, which is
// the goal, so we only have to combine the first 3 to hit the goal
let mut infos = create_test_infos(5);
infos.all_infos[4].alive_bytes = ideal_storage_size_large;
let max_storages = 4;
test(
&mut infos,
max_storages,
Expand All @@ -2520,7 +2542,7 @@ pub mod tests {
.iter()
.map(|info| info.slot)
.collect::<Vec<_>>(),
vec![0, 1]
vec![0, 1, 2, 3, 4]
);
}
}
Expand Down Expand Up @@ -2900,8 +2922,10 @@ pub mod tests {
let active_slots = (0..num_slots)
.filter_map(|slot| db.storage.get_slot_storage_entry((slot as Slot) + slot1))
.count();
let mut expected_slots = max_ancient_slots.min(num_slots);
if max_ancient_slots == 0 {
let mut expected_slots = (max_ancient_slots / 2).min(num_slots);
if max_ancient_slots >= num_slots {
expected_slots = num_slots;
} else if max_ancient_slots == 0 || num_slots > 0 && expected_slots == 0 {
expected_slots = 1;
}
assert_eq!(
Expand Down