Skip to content

Commit

Permalink
Refine the condition for 'should shrink' decision (#3252)
Browse files Browse the repository at this point in the history 
* Refine the condition for 'should shrink' decision

* Fix tests

* Fix clippy

* Brooks
  • Loading branch information
@dmakarov
dmakarov authored Oct 23, 2024
1 parent 0e16e60 commit 5167e0e
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 54 deletions.
2 changes: 1 addition & 1 deletion accounts-db/src/accounts_db.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7543,7 +7543,7 @@ impl AccountsDb {
true
}

fn is_candidate_for_shrink(&self, store: &AccountStorageEntry) -> bool {
pub(crate) fn is_candidate_for_shrink(&self, store: &AccountStorageEntry) -> bool {
// appended ancient append vecs should not be shrunk by the normal shrink codepath.
// It is not possible to identify ancient append vecs when we pack, so no check for ancient when we are not appending.
let total_bytes = if self.create_ancient_storage == CreateAncientStorage::Append
Expand Down
114 changes: 61 additions & 53 deletions accounts-db/src/ancient_append_vecs.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,21 @@ impl AncientSlotInfos {
can_randomly_shrink: bool,
ideal_size: NonZeroU64,
is_high_slot: bool,
is_candidate_for_shrink: bool,
) -> bool {
let mut was_randomly_shrunk = false;
let alive_bytes = storage.alive_bytes() as u64;
if alive_bytes > 0 {
let capacity = storage.accounts.capacity();
let should_shrink = if capacity > 0 {
let alive_ratio = alive_bytes * 100 / capacity;
alive_ratio < 90
|| if can_randomly_shrink && thread_rng().gen_range(0..10000) == 0 {
was_randomly_shrunk = true;
true
} else {
false
}
if is_candidate_for_shrink {
true
} else if can_randomly_shrink && thread_rng().gen_range(0..10000) == 0 {
was_randomly_shrunk = true;
true
} else {
false
}
} else {
false
};
Expand Down Expand Up @@ -575,15 +576,16 @@ impl AccountsDb {
// heuristic to include some # of newly eligible ancient slots so that the pack algorithm always makes progress
let high_slot_boundary = max_slot.saturating_sub(HIGH_SLOT_OFFSET);
let is_high_slot = |slot| slot >= high_slot_boundary;

for slot in &slots {
if let Some(storage) = self.storage.get_slot_storage_entry(*slot) {
let is_candidate_for_shrink = self.is_candidate_for_shrink(&storage);
if infos.add(
*slot,
storage,
tuning.can_randomly_shrink,
tuning.ideal_storage_size,
is_high_slot(*slot),
is_candidate_for_shrink,
) {
randoms += 1;
}
Expand Down Expand Up @@ -2576,6 +2578,7 @@ pub mod tests {
let storage = db.storage.get_slot_storage_entry(slot1).unwrap();
let alive_bytes_expected = storage.alive_bytes();
let high_slot = false;
let is_candidate_for_shrink = db.is_candidate_for_shrink(&storage);
let tuning = PackedAncientStorageTuning {
percent_of_alive_shrunk_data: 100,
max_ancient_slots: 0,
Expand All @@ -2593,6 +2596,7 @@ pub mod tests {
can_randomly_shrink,
NonZeroU64::new(get_ancient_append_vec_capacity()).unwrap(),
high_slot,
is_candidate_for_shrink,
);
}
TestCollectInfo::CalcAncientSlotInfo => {
Expand All @@ -2603,7 +2607,7 @@ pub mod tests {
}
}
assert_eq!(infos.all_infos.len(), 1, "{method:?}");
let should_shrink = data_size.is_none();
let should_shrink = db.is_candidate_for_shrink(&storage);
assert_storage_info(infos.all_infos.first().unwrap(), &storage, should_shrink);
if should_shrink {
// data size is so small compared to min aligned file size that the storage is marked as should_shrink
Expand Down Expand Up @@ -2639,13 +2643,15 @@ pub mod tests {
let mut infos = AncientSlotInfos::default();
let storage = db.storage.get_slot_storage_entry(slot1).unwrap();
let high_slot = false;
let is_candidate_for_shrink = db.is_candidate_for_shrink(&storage);
if call_add {
infos.add(
slot1,
Arc::clone(&storage),
can_randomly_shrink,
NonZeroU64::new(get_ancient_append_vec_capacity()).unwrap(),
high_slot,
is_candidate_for_shrink,
);
} else {
let tuning = PackedAncientStorageTuning {
Expand Down Expand Up @@ -2698,30 +2704,33 @@ pub mod tests {
assert_eq!(infos.total_alive_bytes_shrink.0, 0);
} else {
assert_eq!(infos.all_infos.len(), slots);
let should_shrink = data_size.is_none();
storages
.iter()
.zip(infos.all_infos.iter())
.for_each(|(storage, info)| {
let should_shrink = db.is_candidate_for_shrink(storage);
assert_storage_info(info, storage, should_shrink);
if should_shrink {
// data size is so small compared to min aligned file size that the storage is marked as should_shrink
assert_eq!(
infos.shrink_indexes,
slot_vec
.iter()
.enumerate()
.map(|(i, _)| i)
.collect::<Vec<_>>()
);
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(
infos.total_alive_bytes_shrink.0,
alive_bytes_expected
);
} else {
assert!(infos.shrink_indexes.is_empty());
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(infos.total_alive_bytes_shrink.0, 0);
}
});
if should_shrink {
// data size is so small compared to min aligned file size that the storage is marked as should_shrink
assert_eq!(
infos.shrink_indexes,
slot_vec
.iter()
.enumerate()
.map(|(i, _)| i)
.collect::<Vec<_>>()
);
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(infos.total_alive_bytes_shrink.0, alive_bytes_expected);
} else {
assert!(infos.shrink_indexes.is_empty());
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(infos.total_alive_bytes_shrink.0, 0);
}
}
}
}
Expand Down Expand Up @@ -2795,29 +2804,29 @@ pub mod tests {
}
};
assert_eq!(infos.all_infos.len(), 1, "method: {method:?}");
let should_shrink = data_size.is_none();
alive_storages.iter().zip(infos.all_infos.iter()).for_each(
|(storage, info)| {
let should_shrink = db.is_candidate_for_shrink(storage);
assert_storage_info(info, storage, should_shrink);
},
);
if should_shrink {
// data size is so small compared to min aligned file size that the storage is marked as should_shrink
assert_eq!(
infos.shrink_indexes,
if !matches!(method, TestCollectInfo::CollectSortFilterInfo) {
vec![0]
if should_shrink {
// data size is so small compared to min aligned file size that the storage is marked as should_shrink
assert_eq!(
infos.shrink_indexes,
if !matches!(method, TestCollectInfo::CollectSortFilterInfo) {
vec![0]
} else {
Vec::default()
}
);
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(infos.total_alive_bytes_shrink.0, alive_bytes_expected);
} else {
Vec::default()
assert!(infos.shrink_indexes.is_empty());
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(infos.total_alive_bytes_shrink.0, 0);
}
);
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(infos.total_alive_bytes_shrink.0, alive_bytes_expected);
} else {
assert!(infos.shrink_indexes.is_empty());
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(infos.total_alive_bytes_shrink.0, 0);
}
},
);
}
}
}
Expand Down Expand Up @@ -3173,7 +3182,6 @@ pub mod tests {
.collect::<Vec<_>>();
let (db, slot1) =
create_db_with_storages_and_index(true /*alive*/, 1, data_sizes[1]);
let dead_bytes = 184; // constant based on None data size
create_storages_and_update_index(
&db,
None,
Expand Down Expand Up @@ -3221,19 +3229,19 @@ pub mod tests {
// data size is so small compared to min aligned file size that the storage is marked as should_shrink
assert_eq!(
infos.shrink_indexes,
if matches!(method, TestCollectInfo::CollectSortFilterInfo) {
Vec::default()
} else {
shrinks
match method {
TestCollectInfo::CollectSortFilterInfo => Vec::default(),
TestCollectInfo::CalcAncientSlotInfo => vec![0, 1],
_ => shrinks
.iter()
.skip(1)
.enumerate()
.filter_map(|(i, shrink)| shrink.then_some(i))
.collect::<Vec<_>>()
.collect::<Vec<_>>(),
}
);
assert_eq!(infos.total_alive_bytes.0, alive_bytes_expected);
assert_eq!(infos.total_alive_bytes_shrink.0, dead_bytes);
assert_eq!(infos.total_alive_bytes_shrink.0, alive_bytes_expected);
}
}
}
Expand Down

0 comments on commit 5167e0e

Please sign in to comment.