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.

Sign up for GitHub

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

cleanup ancient append vec tests #29514

Merged
merged 1 commit into from
Jan 4, 2023
Merged

cleanup ancient append vec tests #29514

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
69 changes: 35 additions & 34 deletions runtime/src/accounts_db.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9676,7 +9676,9 @@ pub mod tests {

// there has to be an existing append vec at this slot for a new current ancient at the slot to make sense
let _existing_append_vec = db.create_and_insert_store(slot0, 1000, "test");
let _shrink_in_progress = current_ancient.create_ancient_append_vec(slot0, &db);
{
let _shrink_in_progress = current_ancient.create_ancient_append_vec(slot0, &db);
}
Comment on lines +9679 to +9681
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not always a huge fan of these guards. They're here to force a drop, but are one mental step removed (imo): scope ends -> drop. We could just explicitly drop it, in fewer lines, without the minor mental leap:

Suggested change
{
let _shrink_in_progress = current_ancient.create_ancient_append_vec(slot0, &db);
}
let shrink_in_progress = current_ancient.create_ancient_append_vec(slot0, &db);
drop(shrink_in_progress);

let mut ancient_slot_pubkeys = AncientSlotPubkeys::default();
assert!(ancient_slot_pubkeys.inner.is_none());
// same slot as current_ancient, so no-op
Expand Down Expand Up @@ -16842,9 +16844,15 @@ pub mod tests {
// there has to be an existing append vec at this slot for a new current ancient at the slot to make sense
let _existing_append_vec = db.create_and_insert_store(slot1_ancient, 1000, "test");

let ancient = db.create_ancient_append_vec(slot1_ancient);
let ancient = db
.create_ancient_append_vec(slot1_ancient)
.new_storage()
.clone();
let _existing_append_vec = db.create_and_insert_store(slot1_plus_ancient, 1000, "test");
let ancient_1_plus = db.create_ancient_append_vec(slot1_plus_ancient);
let ancient_1_plus = db
.create_ancient_append_vec(slot1_plus_ancient)
.new_storage()
.clone();
let _existing_append_vec = db.create_and_insert_store(slot3_ancient, 1000, "test");
let ancient3 = db.create_ancient_append_vec(slot3_ancient);
let temp_dir = TempDir::new().unwrap();
Expand All @@ -16865,7 +16873,7 @@ pub mod tests {
assert_eq!(Vec::<Slot>::default(), ancient_slots);

// now test with an ancient append vec
let raw_storages = vec![vec![ancient.new_storage().clone()]];
let raw_storages = vec![vec![ancient.clone()]];
let snapshot_storages = SortedStorages::new(&raw_storages);
let one_epoch_old_slot = 0;
let ancient_slots =
Expand All @@ -16877,10 +16885,7 @@ pub mod tests {
assert_eq!(vec![slot1_ancient], ancient_slots);

// now test with an ancient append vec and then a non-ancient append vec
let raw_storages = vec![
vec![ancient.new_storage().clone()],
vec![non_ancient_storage.clone()],
];
let raw_storages = vec![vec![ancient.clone()], vec![non_ancient_storage.clone()]];
let snapshot_storages = SortedStorages::new(&raw_storages);
let one_epoch_old_slot = 0;
let ancient_slots =
Expand All @@ -16893,7 +16898,7 @@ pub mod tests {

// ancient, non-ancient, ancient
let raw_storages = vec![
vec![ancient.new_storage().clone()],
vec![ancient.clone()],
vec![non_ancient_storage.clone()],
vec![ancient3.new_storage().clone()],
];
Expand All @@ -16910,8 +16915,8 @@ pub mod tests {
if sparse {
// ancient, ancient, non-ancient, ancient
let raw_storages = vec![
vec![Arc::clone(ancient.new_storage())],
vec![Arc::clone(ancient_1_plus.new_storage())],
vec![Arc::clone(&ancient)],
vec![Arc::clone(&ancient_1_plus)],
vec![non_ancient_storage],
vec![Arc::clone(ancient3.new_storage())],
];
Expand Down Expand Up @@ -17081,7 +17086,9 @@ pub mod tests {
// there has to be an existing append vec at this slot for a new current ancient at the slot to make sense
let _existing_append_vec = db.create_and_insert_store(slot2, 1000, "test");

let mut _shrink_in_progress = current_ancient.create_ancient_append_vec(slot2, &db);
{
let _shrink_in_progress = current_ancient.create_ancient_append_vec(slot2, &db);
}
let id = current_ancient.append_vec_id();
assert_eq!(current_ancient.slot(), slot2);
assert!(is_ancient(&current_ancient.append_vec().accounts));
Expand Down Expand Up @@ -17860,40 +17867,39 @@ pub mod tests {
let slot1_ancient = 1;
// there has to be an existing append vec at this slot for a new current ancient at the slot to make sense
let _existing_append_vec = db.create_and_insert_store(slot1_ancient, 1000, "test");
let ancient1 = db.create_ancient_append_vec(slot1_ancient);
let ancient1 = db
.create_ancient_append_vec(slot1_ancient)
.new_storage()
.clone();
let should_move = db.should_move_to_ancient_append_vec(
&ancient1.new_storage().clone(),
&ancient1,
&mut current_ancient,
slot1_ancient,
CAN_RANDOMLY_SHRINK_FALSE,
);
assert!(!should_move);
assert_eq!(
current_ancient.append_vec_id(),
ancient1.new_storage().append_vec_id()
);
assert_eq!(current_ancient.append_vec_id(), ancient1.append_vec_id());
assert_eq!(current_ancient.slot(), slot1_ancient);

// current is ancient1
// try to move ancient2
// current should become ancient2
let slot2_ancient = 2;
let mut current_ancient =
CurrentAncientAppendVec::new(slot1_ancient, ancient1.new_storage().clone());
let mut current_ancient = CurrentAncientAppendVec::new(slot1_ancient, ancient1.clone());
// there has to be an existing append vec at this slot for a new current ancient at the slot to make sense
let _existing_append_vec = db.create_and_insert_store(slot2_ancient, 1000, "test");
let ancient2 = db.create_ancient_append_vec(slot2_ancient);
let ancient2 = db
.create_ancient_append_vec(slot2_ancient)
.new_storage()
.clone();
let should_move = db.should_move_to_ancient_append_vec(
&ancient2.new_storage().clone(),
&ancient2,
&mut current_ancient,
slot2_ancient,
CAN_RANDOMLY_SHRINK_FALSE,
);
assert!(!should_move);
assert_eq!(
current_ancient.append_vec_id(),
ancient2.new_storage().append_vec_id()
);
assert_eq!(current_ancient.append_vec_id(), ancient2.append_vec_id());
assert_eq!(current_ancient.slot(), slot2_ancient);

// now try a full ancient append vec
Expand All @@ -17917,8 +17923,7 @@ pub mod tests {
assert_eq!(current_ancient.slot(), slot3_full_ancient);

// now set current_ancient to something
let mut current_ancient =
CurrentAncientAppendVec::new(slot1_ancient, ancient1.new_storage().clone());
let mut current_ancient = CurrentAncientAppendVec::new(slot1_ancient, ancient1.clone());
let should_move = db.should_move_to_ancient_append_vec(
&full_ancient_3.new_storage().clone(),
&mut current_ancient,
Expand Down Expand Up @@ -17948,19 +17953,15 @@ pub mod tests {

// should return true here, returning current from prior
// now set current_ancient to something and see if it still goes to None
let mut current_ancient =
CurrentAncientAppendVec::new(slot1_ancient, ancient1.new_storage().clone());
let mut current_ancient = CurrentAncientAppendVec::new(slot1_ancient, ancient1.clone());
let should_move = db.should_move_to_ancient_append_vec(
&Arc::clone(full_ancient_3.new_storage()),
&mut current_ancient,
slot3_full_ancient,
CAN_RANDOMLY_SHRINK_FALSE,
);
assert!(should_move);
assert_eq!(
current_ancient.append_vec_id(),
ancient1.new_storage().append_vec_id()
);
assert_eq!(current_ancient.append_vec_id(), ancient1.append_vec_id());
assert_eq!(current_ancient.slot(), slot1_ancient);
}

Expand Down