Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

uses Option<Slot> for SlotMeta.parent_slot (backport #21808) #21899

Merged
mergify merged 2 commits into from
Dec 14, 2021
Merged

uses Option<Slot> for SlotMeta.parent_slot (backport #21808) #21899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9265196
uses Option<Slot> for SlotMeta.parent_slot (#21808)
behzadnouri Dec 14, 2021
717c99f
removes backport merge conflicts
behzadnouri Dec 14, 2021
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
12 changes: 6 additions & 6 deletions core/src/repair_generic_traversal.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,17 @@ fn get_unrepaired_path(
) -> Vec<Slot> {
let mut path = Vec::new();
let mut slot = start_slot;
while !visited.contains(&slot) {
visited.insert(slot);
while visited.insert(slot) {
let slot_meta = slot_meta_cache
.entry(slot)
.or_insert_with(|| blockstore.meta(slot).unwrap());
if let Some(slot_meta) = slot_meta {
if slot_meta.is_full() {
break;
if !slot_meta.is_full() {
path.push(slot);
if let Some(parent_slot) = slot_meta.parent_slot {
slot = parent_slot
}
}
path.push(slot);
slot = slot_meta.parent_slot;
}
}
path.reverse();
Expand Down
4 changes: 2 additions & 2 deletions core/src/serve_repair.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,8 +706,8 @@ impl ServeRepair {
} else {
break;
}
if meta.is_parent_set() && res.packets.len() <= max_responses {
slot = meta.parent_slot;
if meta.parent_slot.is_some() && res.packets.len() <= max_responses {
slot = meta.parent_slot.unwrap();
} else {
break;
}
Expand Down
2 changes: 1 addition & 1 deletion ledger-tool/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ fn output_slot(
println!(" Slot Meta {:?} is_full: {}", meta, is_full);
} else {
println!(
" num_shreds: {} parent_slot: {} num_entries: {} is_full: {}",
" num_shreds: {}, parent_slot: {:?}, num_entries: {}, is_full: {}",
num_shreds,
meta.parent_slot,
entries.len(),
Expand Down
16 changes: 7 additions & 9 deletions ledger/src/ancestor_iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ pub struct AncestorIterator<'a> {
impl<'a> AncestorIterator<'a> {
pub fn new(start_slot: Slot, blockstore: &'a Blockstore) -> Self {
let current = blockstore.meta(start_slot).unwrap().and_then(|slot_meta| {
if slot_meta.is_parent_set() && start_slot != 0 {
Some(slot_meta.parent_slot)
if start_slot != 0 {
slot_meta.parent_slot
} else {
None
}
Expand All @@ -37,13 +37,11 @@ impl<'a> Iterator for AncestorIterator<'a> {
let current = self.current;
current.map(|slot| {
if slot != 0 {
self.current = self.blockstore.meta(slot).unwrap().and_then(|slot_meta| {
if slot_meta.is_parent_set() {
Some(slot_meta.parent_slot)
} else {
None
}
});
self.current = self
.blockstore
.meta(slot)
.unwrap()
.and_then(|slot_meta| slot_meta.parent_slot);
} else {
self.current = None;
}
Expand Down
104 changes: 56 additions & 48 deletions ledger/src/blockstore.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1410,7 +1410,12 @@ impl Blockstore {
}

let last_root = *last_root.read().unwrap();
verify_shred_slots(slot, slot_meta.parent_slot, last_root)
// TODO Shouldn't this use shred.parent() instead and update
// slot_meta.parent_slot accordingly?
slot_meta
.parent_slot
.map(|parent_slot| verify_shred_slots(slot, parent_slot, last_root))
.unwrap_or_default()
}

fn insert_data_shred(
Expand Down Expand Up @@ -1869,8 +1874,12 @@ impl Blockstore {
}
transaction
});
let parent_slot_entries = self
.get_slot_entries(slot_meta.parent_slot, 0)
let parent_slot_entries = slot_meta
.parent_slot
.and_then(|parent_slot| {
self.get_slot_entries(parent_slot, /*shred_start_index:*/ 0)
.ok()
})
.unwrap_or_default();
if parent_slot_entries.is_empty() && require_previous_blockhash {
return Err(BlockstoreError::ParentEntriesUnavailable);
Expand All @@ -1896,7 +1905,9 @@ impl Blockstore {
let block = ConfirmedBlock {
previous_blockhash: previous_blockhash.to_string(),
blockhash: blockhash.to_string(),
parent_slot: slot_meta.parent_slot,
// If the slot is full it should have parent_slot populated
// from shreds received.
parent_slot: slot_meta.parent_slot.unwrap(),
transactions: self
.map_transactions_to_statuses(slot, slot_transaction_iterator)?,
rewards,
Expand Down Expand Up @@ -3227,18 +3238,18 @@ fn get_slot_meta_entry<'a>(
// Store a 2-tuple of the metadata (working copy, backup copy)
if let Some(mut meta) = meta_cf.get(slot).expect("Expect database get to succeed") {
let backup = Some(meta.clone());
// If parent_slot == std::u64::MAX, then this is one of the orphans inserted
// If parent_slot == None, then this is one of the orphans inserted
// during the chaining process, see the function find_slot_meta_in_cached_state()
// for details. Slots that are orphans are missing a parent_slot, so we should
// fill in the parent now that we know it.
if is_orphan(&meta) {
meta.parent_slot = parent_slot;
meta.parent_slot = Some(parent_slot);
}

SlotMetaWorkingSetEntry::new(Rc::new(RefCell::new(meta)), backup)
} else {
SlotMetaWorkingSetEntry::new(
Rc::new(RefCell::new(SlotMeta::new(slot, parent_slot))),
Rc::new(RefCell::new(SlotMeta::new(slot, Some(parent_slot)))),
None,
)
}
Expand Down Expand Up @@ -3409,8 +3420,8 @@ fn handle_chaining_for_slot(
// 1) This is a new slot
// 2) slot != 0
// then try to chain this slot to a previous slot
if slot != 0 {
let prev_slot = meta_mut.parent_slot;
if slot != 0 && meta_mut.parent_slot.is_some() {
let prev_slot = meta_mut.parent_slot.unwrap();

// Check if the slot represented by meta_mut is either a new slot or a orphan.
// In both cases we need to run the chaining logic b/c the parent on the slot was
Expand Down Expand Up @@ -3504,7 +3515,7 @@ where
fn is_orphan(meta: &SlotMeta) -> bool {
// If we have no parent, then this is the head of a detached chain of
// slots
!meta.is_parent_set()
meta.parent_slot.is_none()
}

// 1) Chain current_slot to the previous slot defined by prev_slot_meta
Expand Down Expand Up @@ -4027,9 +4038,9 @@ pub mod tests {
assert_eq!(meta.next_slots, vec![i + 1]);
}
if i == 0 {
assert_eq!(meta.parent_slot, 0);
assert_eq!(meta.parent_slot, Some(0));
} else {
assert_eq!(meta.parent_slot, i - 1);
assert_eq!(meta.parent_slot, Some(i - 1));
}

assert_eq!(
Expand Down Expand Up @@ -4087,7 +4098,7 @@ pub mod tests {
let blockstore = Blockstore::open(ledger_path.path()).unwrap();

// Test meta column family
let meta = SlotMeta::new(0, 1);
let meta = SlotMeta::new(0, Some(1));
blockstore.meta_cf.put(0, &meta).unwrap();
let result = blockstore
.meta_cf
Expand Down Expand Up @@ -4244,7 +4255,7 @@ pub mod tests {
.expect("Expected new metadata object to exist");
assert_eq!(meta.consumed, num_shreds);
assert_eq!(meta.received, num_shreds);
assert_eq!(meta.parent_slot, 0);
assert_eq!(meta.parent_slot, Some(0));
assert_eq!(meta.last_index, Some(num_shreds - 1));
assert!(meta.next_slots.is_empty());
assert!(meta.is_connected);
Expand Down Expand Up @@ -4275,7 +4286,7 @@ pub mod tests {
assert_eq!(result.len(), 0);
assert!(meta.consumed == 0 && meta.received == num_shreds as u64);
} else {
assert_eq!(meta.parent_slot, 0);
assert_eq!(meta.parent_slot, Some(0));
assert_eq!(result, entries);
assert!(meta.consumed == num_shreds as u64 && meta.received == num_shreds as u64);
}
Expand Down Expand Up @@ -4463,7 +4474,7 @@ pub mod tests {
let meta = blockstore.meta(slot).unwrap().unwrap();
assert_eq!(meta.received, num_shreds);
assert_eq!(meta.consumed, num_shreds);
assert_eq!(meta.parent_slot, parent_slot);
assert_eq!(meta.parent_slot, Some(parent_slot));
assert_eq!(meta.last_index, Some(num_shreds - 1));
}
}
Expand Down Expand Up @@ -4717,7 +4728,7 @@ pub mod tests {
assert!(s1.next_slots.is_empty());
// Slot 1 is not trunk because slot 0 hasn't been inserted yet
assert!(!s1.is_connected);
assert_eq!(s1.parent_slot, 0);
assert_eq!(s1.parent_slot, Some(0));
assert_eq!(s1.last_index, Some(shreds_per_slot as u64 - 1));

// 2) Write to the second slot
Expand All @@ -4729,15 +4740,15 @@ pub mod tests {
assert!(s2.next_slots.is_empty());
// Slot 2 is not trunk because slot 0 hasn't been inserted yet
assert!(!s2.is_connected);
assert_eq!(s2.parent_slot, 1);
assert_eq!(s2.parent_slot, Some(1));
assert_eq!(s2.last_index, Some(shreds_per_slot as u64 - 1));

// Check the first slot again, it should chain to the second slot,
// but still isn't part of the trunk
let s1 = blockstore.meta(1).unwrap().unwrap();
assert_eq!(s1.next_slots, vec![2]);
assert!(!s1.is_connected);
assert_eq!(s1.parent_slot, 0);
assert_eq!(s1.parent_slot, Some(0));
assert_eq!(s1.last_index, Some(shreds_per_slot as u64 - 1));

// 3) Write to the zeroth slot, check that every slot
Expand All @@ -4750,9 +4761,9 @@ pub mod tests {
assert_eq!(s.next_slots, vec![i + 1]);
}
if i == 0 {
assert_eq!(s.parent_slot, 0);
assert_eq!(s.parent_slot, Some(0));
} else {
assert_eq!(s.parent_slot, i - 1);
assert_eq!(s.parent_slot, Some(i - 1));
}
assert_eq!(s.last_index, Some(shreds_per_slot as u64 - 1));
assert!(s.is_connected);
Expand Down Expand Up @@ -4802,10 +4813,10 @@ pub mod tests {
let s = blockstore.meta(i as u64).unwrap().unwrap();
if i % 2 == 0 {
assert_eq!(s.next_slots, vec![i as u64 + 1]);
assert_eq!(s.parent_slot, std::u64::MAX);
assert_eq!(s.parent_slot, None);
} else {
assert!(s.next_slots.is_empty());
assert_eq!(s.parent_slot, i - 1);
assert_eq!(s.parent_slot, Some(i - 1));
}

if i == 0 {
Expand All @@ -4831,9 +4842,9 @@ pub mod tests {
}

if i == 0 {
assert_eq!(s.parent_slot, 0);
assert_eq!(s.parent_slot, Some(0));
} else {
assert_eq!(s.parent_slot, i - 1);
assert_eq!(s.parent_slot, Some(i - 1));
}
assert_eq!(s.last_index, Some(shreds_per_slot as u64 - 1));
assert!(s.is_connected);
Expand Down Expand Up @@ -4879,9 +4890,9 @@ pub mod tests {
}

if i == 0 {
assert_eq!(s.parent_slot, 0);
assert_eq!(s.parent_slot, Some(0));
} else {
assert_eq!(s.parent_slot, i - 1);
assert_eq!(s.parent_slot, Some(i - 1));
}

assert_eq!(s.last_index, Some(shreds_per_slot as u64 - 1));
Expand Down Expand Up @@ -4915,9 +4926,9 @@ pub mod tests {
}

if i == 0 {
assert_eq!(s.parent_slot, 0);
assert_eq!(s.parent_slot, Some(0));
} else {
assert_eq!(s.parent_slot, i - 1);
assert_eq!(s.parent_slot, Some(i - 1));
}

assert_eq!(s.last_index, Some(shreds_per_slot as u64 - 1));
Expand Down Expand Up @@ -4999,7 +5010,7 @@ pub mod tests {
(slot - 1) / branching_factor
}
};
assert_eq!(slot_meta.parent_slot, slot_parent);
assert_eq!(slot_meta.parent_slot, Some(slot_parent));

let expected_children: HashSet<_> = {
if slot >= last_level {
Expand Down Expand Up @@ -5033,7 +5044,7 @@ pub mod tests {
// Slot doesn't exist
assert!(blockstore.get_slots_since(&[0]).unwrap().is_empty());

let mut meta0 = SlotMeta::new(0, 0);
let mut meta0 = SlotMeta::new(0, Some(0));
blockstore.meta_cf.put(0, &meta0).unwrap();

// Slot exists, chains to nothing
Expand All @@ -5047,7 +5058,7 @@ pub mod tests {
assert_eq!(blockstore.get_slots_since(&[0]).unwrap(), expected);
assert_eq!(blockstore.get_slots_since(&[0, 1]).unwrap(), expected);

let mut meta3 = SlotMeta::new(3, 1);
let mut meta3 = SlotMeta::new(3, Some(1));
meta3.next_slots = vec![10, 5];
blockstore.meta_cf.put(3, &meta3).unwrap();
let expected: HashMap<u64, Vec<u64>> = vec![(0, vec![1, 2]), (3, vec![10, 5])]
Expand Down Expand Up @@ -5176,10 +5187,10 @@ pub mod tests {
assert_eq!(meta.received, 1);
assert_eq!(meta.last_index, Some(0));
if i != 0 {
assert_eq!(meta.parent_slot, i - 1);
assert_eq!(meta.parent_slot, Some(i - 1));
assert_eq!(meta.consumed, 1);
} else {
assert_eq!(meta.parent_slot, 0);
assert_eq!(meta.parent_slot, Some(0));
assert_eq!(meta.consumed, num_shreds_per_slot);
}
}
Expand Down Expand Up @@ -6005,10 +6016,7 @@ pub mod tests {
.set_roots(vec![slot - 1, slot, slot + 1].iter())
.unwrap();

let parent_meta = SlotMeta {
parent_slot: std::u64::MAX,
..SlotMeta::default()
};
let parent_meta = SlotMeta::default();
blockstore
.put_meta_bytes(slot - 1, &serialize(&parent_meta).unwrap())
.unwrap();
Expand Down Expand Up @@ -6543,13 +6551,13 @@ pub mod tests {
// 2 (root)
// |
// 3
let meta0 = SlotMeta::new(0, 0);
let meta0 = SlotMeta::new(0, Some(0));
blockstore.meta_cf.put(0, &meta0).unwrap();
let meta1 = SlotMeta::new(1, 0);
let meta1 = SlotMeta::new(1, Some(0));
blockstore.meta_cf.put(1, &meta1).unwrap();
let meta2 = SlotMeta::new(2, 0);
let meta2 = SlotMeta::new(2, Some(0));
blockstore.meta_cf.put(2, &meta2).unwrap();
let meta3 = SlotMeta::new(3, 2);
let meta3 = SlotMeta::new(3, Some(2));
blockstore.meta_cf.put(3, &meta3).unwrap();

blockstore.set_roots(vec![0, 2].iter()).unwrap();
Expand Down Expand Up @@ -6726,13 +6734,13 @@ pub mod tests {
let signature2 = Signature::new(&[3u8; 64]);

// Insert rooted slots 0..=3 with no fork
let meta0 = SlotMeta::new(0, 0);
let meta0 = SlotMeta::new(0, Some(0));
blockstore.meta_cf.put(0, &meta0).unwrap();
let meta1 = SlotMeta::new(1, 0);
let meta1 = SlotMeta::new(1, Some(0));
blockstore.meta_cf.put(1, &meta1).unwrap();
let meta2 = SlotMeta::new(2, 1);
let meta2 = SlotMeta::new(2, Some(1));
blockstore.meta_cf.put(2, &meta2).unwrap();
let meta3 = SlotMeta::new(3, 2);
let meta3 = SlotMeta::new(3, Some(2));
blockstore.meta_cf.put(3, &meta3).unwrap();

blockstore.set_roots(vec![0, 1, 2, 3].iter()).unwrap();
Expand Down Expand Up @@ -8442,7 +8450,7 @@ pub mod tests {
assert_eq!(blockstore.get_slot_entries(0, 0).unwrap(), vec![]);
assert_eq!(meta.consumed, 0);
assert_eq!(meta.received, last_index + 1);
assert_eq!(meta.parent_slot, 0);
assert_eq!(meta.parent_slot, Some(0));
assert_eq!(meta.last_index, Some(last_index));
assert!(!blockstore.is_full(0));
}
Expand All @@ -8458,7 +8466,7 @@ pub mod tests {
let meta = blockstore.meta(0).unwrap().unwrap();
assert_eq!(meta.consumed, num_shreds);
assert_eq!(meta.received, num_shreds);
assert_eq!(meta.parent_slot, 0);
assert_eq!(meta.parent_slot, Some(0));
assert_eq!(meta.last_index, Some(num_shreds - 1));
assert!(blockstore.is_full(0));
assert!(!blockstore.is_dead(0));
Expand Down
Loading