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

Change SlotMeta is_connected bool to bitflags #29001

Merged
merged 1 commit into from
Dec 1, 2022
Merged

Change SlotMeta is_connected bool to bitflags #29001

Show file tree
Hide file tree
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
36 changes: 19 additions & 17 deletions ledger/src/blockstore.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3730,13 +3730,13 @@ fn handle_chaining_for_slot(
// connected to trunk of the ledger
let should_propagate_is_connected =
is_newly_completed_slot(&RefCell::borrow(meta), meta_backup)
&& RefCell::borrow(meta).is_connected;
&& RefCell::borrow(meta).is_connected();

if should_propagate_is_connected {
// slot_function returns a boolean indicating whether to explore the children
// of the input slot
let slot_function = |slot: &mut SlotMeta| {
slot.is_connected = true;
slot.set_connected();

// We don't want to set the is_connected flag on the children of non-full
// slots
Expand Down Expand Up @@ -3817,7 +3817,9 @@ fn chain_new_slot_to_prev_slot(
current_slot_meta: &mut SlotMeta,
) {
prev_slot_meta.next_slots.push(current_slot);
current_slot_meta.is_connected = prev_slot_meta.is_connected && prev_slot_meta.is_full();
if prev_slot_meta.is_connected() && prev_slot_meta.is_full() {
current_slot_meta.set_connected();
}
}

fn is_newly_completed_slot(slot_meta: &SlotMeta, backup_slot_meta: &Option<SlotMeta>) -> bool {
Expand All @@ -3831,7 +3833,7 @@ fn slot_has_updates(slot_meta: &SlotMeta, slot_meta_backup: &Option<SlotMeta>) -
// from block 0, which is true iff:
// 1) The block with index prev_block_index is itself part of the trunk of consecutive blocks
// starting from block 0,
slot_meta.is_connected &&
slot_meta.is_connected() &&
// AND either:
// 1) The slot didn't exist in the database before, and now we have a consecutive
// block for that slot
Expand Down Expand Up @@ -4819,7 +4821,7 @@ pub mod tests {
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);
assert!(meta.is_connected());
}

#[test]
Expand Down Expand Up @@ -5342,7 +5344,7 @@ pub mod tests {
let meta1 = blockstore.meta(1).unwrap().unwrap();
assert!(meta1.next_slots.is_empty());
// Slot 1 is not trunk because slot 0 hasn't been inserted yet
assert!(!meta1.is_connected);
assert!(!meta1.is_connected());
assert_eq!(meta1.parent_slot, Some(0));
assert_eq!(meta1.last_index, Some(shreds_per_slot as u64 - 1));

Expand All @@ -5354,15 +5356,15 @@ pub mod tests {
let meta2 = blockstore.meta(2).unwrap().unwrap();
assert!(meta2.next_slots.is_empty());
// Slot 2 is not trunk because slot 0 hasn't been inserted yet
assert!(!meta2.is_connected);
assert!(!meta2.is_connected());
assert_eq!(meta2.parent_slot, Some(1));
assert_eq!(meta2.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 meta1 = blockstore.meta(1).unwrap().unwrap();
assert_eq!(meta1.next_slots, vec![2]);
assert!(!meta1.is_connected);
assert!(!meta1.is_connected());
assert_eq!(meta1.parent_slot, Some(0));
assert_eq!(meta1.last_index, Some(shreds_per_slot as u64 - 1));

Expand All @@ -5381,7 +5383,7 @@ pub mod tests {
assert_eq!(meta.parent_slot, Some(slot - 1));
}
assert_eq!(meta.last_index, Some(shreds_per_slot as u64 - 1));
assert!(meta.is_connected);
assert!(meta.is_connected());
}
}

Expand Down Expand Up @@ -5440,9 +5442,9 @@ pub mod tests {
}

if slot == 0 {
assert!(meta.is_connected);
assert!(meta.is_connected());
} else {
assert!(!meta.is_connected);
assert!(!meta.is_connected());
}
}

Expand All @@ -5467,7 +5469,7 @@ pub mod tests {
assert_eq!(meta.parent_slot, Some(slot - 1));
}
assert_eq!(meta.last_index, Some(shreds_per_slot as u64 - 1));
assert!(meta.is_connected);
assert!(meta.is_connected());
}
}

Expand Down Expand Up @@ -5513,10 +5515,10 @@ pub mod tests {
// Additionally, slot 0 should be the only connected slot
if slot == 0 {
assert_eq!(meta.parent_slot, Some(0));
assert!(meta.is_connected);
assert!(meta.is_connected());
} else {
assert_eq!(meta.parent_slot, Some(slot - 1));
assert!(!meta.is_connected);
assert!(!meta.is_connected());
}

assert_eq!(meta.last_index, Some(shreds_per_slot as u64 - 1));
Expand All @@ -5537,9 +5539,9 @@ pub mod tests {
assert!(meta.next_slots.is_empty());
}
if slot <= slot_index + 3 {
assert!(meta.is_connected);
assert!(meta.is_connected());
} else {
assert!(!meta.is_connected);
assert!(!meta.is_connected());
}

if slot == 0 {
Expand Down Expand Up @@ -5619,7 +5621,7 @@ pub mod tests {
let slot_meta = blockstore.meta(slot).unwrap().unwrap();
assert_eq!(slot_meta.consumed, entries_per_slot);
assert_eq!(slot_meta.received, entries_per_slot);
assert!(slot_meta.is_connected);
assert!(slot_meta.is_connected());
let slot_parent = {
if slot == 0 {
0
Expand Down
133 changes: 129 additions & 4 deletions ledger/src/blockstore_meta.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use {
crate::shred::{Shred, ShredType},
bitflags::bitflags,
serde::{Deserialize, Deserializer, Serialize, Serializer},
solana_sdk::{
clock::{Slot, UnixTimestamp},
Expand All @@ -11,6 +12,43 @@ use {
},
};

bitflags! {
#[derive(Deserialize, Serialize)]
/// Flags to indicate whether a slot is a descendant of a slot on the main fork
pub struct ConnectedFlags:u8 {
// A slot S should be considered to be connected if:
// 1) S is a rooted slot itself OR
// 2) S's parent is connected AND S is full (S's complete block present)
//
// 1) is a straightfoward case, roots are finalized blocks on the main fork
// so by definition, they are connected. All roots are connected, but not
// all connected slots are (or will become) roots.
//
// Based on the criteria stated in 2), S is connected iff it has a series
// of ancestors (that are each connected) that form a chain back to
// some root slot.
//
// A ledger that is updating with a cluster will have either begun at
// genesis or at at some snapshot slot.
// - Genesis is obviously a special case, and slot 0's parent is deemed
// to be connected in order to kick off the induction
// - Snapshots are taken at rooted slots, and as such, the snapshot slot
// should be marked as connected so that a connected chain can start
//
// CONNECTED is explicitly the first bit to ensure backwards compatibility
// with the boolean field that ConnectedFlags replaced in SlotMeta.
const CONNECTED = 0b0000_0001;
steviez marked this conversation as resolved.
Show resolved Hide resolved
// PARENT_CONNECTED IS INTENTIIONALLY UNUSED FOR NOW
const PARENT_CONNECTED = 0b1000_0000;
}
}

impl Default for ConnectedFlags {
fn default() -> Self {
ConnectedFlags::empty()
}
}

#[derive(Clone, Debug, Default, Deserialize, Serialize, Eq, PartialEq)]
// The Meta column family
pub struct SlotMeta {
Expand All @@ -37,9 +75,8 @@ pub struct SlotMeta {
// The list of slots, each of which contains a block that derives
// from this one.
pub next_slots: Vec<Slot>,
// True if this slot is full (consumed == last_index + 1) and if every
// slot that is a parent of this slot is also connected.
pub is_connected: bool,
// Connected status flags of this slot
pub connected_flags: ConnectedFlags,
// Shreds indices which are marked data complete.
pub completed_data_indexes: BTreeSet<u32>,
}
Expand Down Expand Up @@ -214,6 +251,14 @@ impl SlotMeta {
Some(self.consumed) == self.last_index.map(|ix| ix + 1)
}

pub fn is_connected(&self) -> bool {
self.connected_flags.contains(ConnectedFlags::CONNECTED)
}

pub fn set_connected(&mut self) {
self.connected_flags.set(ConnectedFlags::CONNECTED, true);
}

/// Dangerous. Currently only needed for a local-cluster test
pub fn unset_parent(&mut self) {
self.parent_slot = None;
Expand All @@ -225,10 +270,17 @@ impl SlotMeta {
}

pub(crate) fn new(slot: Slot, parent_slot: Option<Slot>) -> Self {
let connected_flags = if slot == 0 {
// Slot 0 is the start, mark it as having its' parent connected
// such that slot 0 becoming full will be updated as connected
ConnectedFlags::CONNECTED
steviez marked this conversation as resolved.
Show resolved Hide resolved
} else {
ConnectedFlags::default()
};
SlotMeta {
slot,
parent_slot,
is_connected: slot == 0,
connected_flags,
..SlotMeta::default()
}
}
Expand Down Expand Up @@ -426,6 +478,79 @@ mod test {
}
}

#[test]
fn test_connected_flags_compatibility() {
steviez marked this conversation as resolved.
Show resolved Hide resolved
// Define a couple structs with bool and ConnectedFlags to illustrate
// that that ConnectedFlags can be deserialized into a bool if the
// PARENT_CONNECTED bit is NOT set
#[derive(Debug, Deserialize, PartialEq, Serialize)]
struct WithBool {
slot: Slot,
connected: bool,
}
#[derive(Debug, Deserialize, PartialEq, Serialize)]
struct WithFlags {
slot: Slot,
connected: ConnectedFlags,
}

let slot = 3;
let mut with_bool = WithBool {
slot,
connected: false,
};
let mut with_flags = WithFlags {
slot,
connected: ConnectedFlags::default(),
};

// Confirm that serialized byte arrays are same length
assert_eq!(
bincode::serialized_size(&with_bool).unwrap(),
bincode::serialized_size(&with_flags).unwrap()
);

// Confirm that connected=false equivalent to ConnectedFlags::default()
assert_eq!(
bincode::serialize(&with_bool).unwrap(),
bincode::serialize(&with_flags).unwrap()
);

// Set connected in WithBool and confirm inequality
with_bool.connected = true;
assert_ne!(
bincode::serialize(&with_bool).unwrap(),
bincode::serialize(&with_flags).unwrap()
);

// Set connected in WithFlags and confirm equality regained
with_flags.connected.set(ConnectedFlags::CONNECTED, true);
assert_eq!(
bincode::serialize(&with_bool).unwrap(),
bincode::serialize(&with_flags).unwrap()
);

// Dserializing WithBool into WithFlags succeeds
assert_eq!(
with_flags,
bincode::deserialize::<WithFlags>(&bincode::serialize(&with_bool).unwrap()).unwrap()
);

// Deserializing WithFlags into WithBool succeeds
assert_eq!(
with_bool,
bincode::deserialize::<WithBool>(&bincode::serialize(&with_flags).unwrap()).unwrap()
);

// Deserializing WithFlags with extra bit set into WithBool fails
with_flags
.connected
.set(ConnectedFlags::PARENT_CONNECTED, true);
assert!(
bincode::deserialize::<WithBool>(&bincode::serialize(&with_flags).unwrap()).is_err()
);
}

#[test]
fn test_clear_unconfirmed_slot() {
let mut slot_meta = SlotMeta::new_orphan(5);
Expand Down