Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
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
57 changes: 56 additions & 1 deletion ledger/src/blockstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ impl Blockstore {
}

fn erasure_meta(&self, erasure_set: ErasureSetId) -> Result<Option<ErasureMeta>> {
self.erasure_meta_cf.get(erasure_set.store_key())
self.erasure_meta_cf.get_new_or_old(erasure_set.store_key())
}

/// Check whether the specified slot is an orphan slot which does not
Expand Down Expand Up @@ -7504,6 +7504,61 @@ pub mod tests {
assert_eq!(meta.fee, index1_slot * 1000);
}

#[test]
#[allow(deprecated)]
fn test_erasure_meta_migration() {
let ledger_path = get_tmp_ledger_path_auto_delete!();
let blockstore = Blockstore::open(ledger_path.path()).unwrap();
let erasure_meta_cf = &blockstore.erasure_meta_cf;

let config = ErasureConfig::new(1, 17);
let erasure_meta_old = ErasureMetaLegacy {
set_index: 5,
first_coding_index: 8,
config,
__unused_size: 0,
};

erasure_meta_cf
.put_old_type((100, 5), &erasure_meta_old)
.unwrap();

let erasure_meta = erasure_meta_cf.get_new_or_old((100, 5)).unwrap().unwrap();
assert_eq!(erasure_meta.set_index(), erasure_meta_old.set_index);
assert_eq!(
erasure_meta.first_coding_index(),
erasure_meta_old.first_coding_index
);
assert_eq!(erasure_meta.config(), erasure_meta_old.config);
assert_eq!(
erasure_meta.first_received_coding_index(),
erasure_meta_old.first_coding_index
);
assert_eq!(erasure_meta.merkle_root(), Hash::default());

let erasure_meta_new = ErasureMetaLegacy {
set_index: 3,
first_coding_index: 2,
config,
__unused_size: 0,
}
.into();
erasure_meta_cf.put((101, 3), &erasure_meta_new).unwrap();

let erasure_meta = erasure_meta_cf.get_new_or_old((101, 3)).unwrap().unwrap();
assert_eq!(erasure_meta.set_index(), erasure_meta_new.set_index());
assert_eq!(
erasure_meta.first_coding_index(),
erasure_meta_new.first_coding_index()
);
assert_eq!(erasure_meta.config(), erasure_meta_new.config());
assert_eq!(
erasure_meta.first_received_coding_index(),
erasure_meta_new.first_coding_index()
);
assert_eq!(erasure_meta.merkle_root(), Hash::default());
}

#[test]
fn test_get_transaction_status() {
let ledger_path = get_tmp_ledger_path_auto_delete!();
Expand Down
59 changes: 58 additions & 1 deletion ledger/src/blockstore_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ pub mod columns {
///
/// This column family stores ErasureMeta which includes metadata about
/// dropped network packets (or erasures) that can be used to recover
/// missing data shreds.
/// missing data shreds. For merkle shreds, it also stores the merkle root.
///
/// Its index type is `crate::shred::ErasureSetId`, which consists of a Slot ID
/// and a FEC (Forward Error Correction) set index.
Expand Down Expand Up @@ -802,6 +802,12 @@ pub trait ColumnIndexDeprecation: Column {
}
}

/// Helper trait to transition a column between bincode formats
pub trait BincodeTypeTransition: Column + TypedColumn {
/// This should have a different serialized size than the TypedColumn::Type
type OldType: Serialize + DeserializeOwned + Into<Self::Type>;
}

Comment on lines +805 to +810
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This type trait and its associated code is pretty verbose and actually adding more complexity here.

Might be simple just to do something like below:

diff --git a/ledger/src/blockstore_db.rs b/ledger/src/blockstore_db.rs
index b65df82ee0..eebf163400 100644
--- a/ledger/src/blockstore_db.rs
+++ b/ledger/src/blockstore_db.rs
@@ -1586,7 +1586,7 @@ where
     }
 }
 
-impl<C> LedgerColumn<C>
+impl<C: 'static> LedgerColumn<C>
 where
     C: TypedColumn + ColumnName,
 {
@@ -1635,7 +1635,12 @@ where
             &self.read_perf_status,
         );
         if let Some(pinnable_slice) = self.backend.get_pinned_cf(self.handle(), key)? {
-            let value = deserialize(pinnable_slice.as_ref())?;
+            use std::any::TypeId;
+            let value = deserialize(pinnable_slice.as_ref()).or_else(|err| {
+                if TypeId::of::<C>() == TypeId::of::<columns::MerkleErasureMeta>() {
+                    // retry with the old erasure meta type.
+                    // ...
+                }
+                Err(err)
+            })?;
             result = Ok(Some(value))
         }
 

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added it to provide an easy way to transition types of already existing columns in blockstore. Similar to ColumnIndexDeprecation.

All you have to do is implement the trait and TypedColumn. And after the old type is no longer in blockstore, you can just remove the trait impl and the associated migration test.

Figured its worth adding the complexity here to make future transitions frictionless.

impl Column for columns::TransactionStatus {
type Index = (Signature, Slot);

Expand Down Expand Up @@ -1218,6 +1224,10 @@ impl ColumnName for columns::ErasureMeta {
impl TypedColumn for columns::ErasureMeta {
type Type = blockstore_meta::ErasureMeta;
}
impl BincodeTypeTransition for columns::ErasureMeta {
#[allow(deprecated)]
type OldType = blockstore_meta::ErasureMetaLegacy;
}

impl SlotColumn for columns::OptimisticSlots {}
impl ColumnName for columns::OptimisticSlots {
Expand Down Expand Up @@ -1796,6 +1806,42 @@ where
}
}

impl<C> LedgerColumn<C>
where
C: BincodeTypeTransition + ColumnName,
{
/// Read the column in either the `C::Type` or `C::OldType` format.
pub fn get_new_or_old(&self, key: C::Index) -> Result<Option<C::Type>> {
self.get_new_or_old_raw(&C::key(key))
}

pub fn get_new_or_old_raw(&self, key: &[u8]) -> Result<Option<C::Type>> {
let mut result = Ok(None);
let is_perf_enabled = maybe_enable_rocksdb_perf(
self.column_options.rocks_perf_sample_interval,
&self.read_perf_status,
);
if let Some(pinnable_slice) = self.backend.get_pinned_cf(self.handle(), key)? {
let value = if let Ok(value) = deserialize::<C::Type>(pinnable_slice.as_ref()) {
value
} else {
deserialize::<C::OldType>(pinnable_slice.as_ref())?.into()
};
result = Ok(Some(value))
}

if let Some(op_start_instant) = is_perf_enabled {
report_rocksdb_read_perf(
C::NAME,
PERF_METRIC_OP_NAME_GET,
&op_start_instant.elapsed(),
&self.column_options,
);
}
result
}
}

impl<'a> WriteBatch<'a> {
pub fn put_bytes<C: Column + ColumnName>(&mut self, key: C::Index, bytes: &[u8]) -> Result<()> {
self.write_batch
Expand Down Expand Up @@ -2236,6 +2282,17 @@ pub mod tests {
}
}

impl<C> LedgerColumn<C>
where
C: BincodeTypeTransition + ColumnName,
{
pub fn put_old_type(&self, key: C::Index, value: &C::OldType) -> Result<()> {
let serialized_value = serialize(value)?;
self.backend
.put_cf(self.handle(), &C::key(key), &serialized_value)
}
}

impl<C> LedgerColumn<C>
where
C: ColumnIndexDeprecation + ColumnName,
Expand Down
78 changes: 72 additions & 6 deletions ledger/src/blockstore_meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,18 +118,34 @@ pub struct ShredIndex {
index: BTreeSet<u64>,
}

#[deprecated = "Use ErasureMeta"]
#[derive(Clone, Copy, Debug, Deserialize, Serialize, Eq, PartialEq)]
/// Erasure coding information
pub struct ErasureMetaLegacy {
/// Which erasure set in the slot this is
pub(crate) set_index: u64,
/// First coding index in the FEC set
pub(crate) first_coding_index: u64,
/// Size of shards in this erasure set
#[serde(rename = "size")]
pub(crate) __unused_size: usize,
/// Erasure configuration for this erasure set
pub(crate) config: ErasureConfig,
}

#[derive(Clone, Copy, Debug, Deserialize, Serialize, Eq, PartialEq)]
/// Erasure coding information for merkle shreds
pub struct ErasureMeta {
/// Which erasure set in the slot this is
set_index: u64,
/// First coding index in the FEC set
first_coding_index: u64,
/// Size of shards in this erasure set
#[serde(rename = "size")]
__unused_size: usize,
/// First coding shred received, from which we populated this ErasureMeta
first_received_coding_index: u64,
/// Erasure configuration for this erasure set
config: ErasureConfig,
/// Merkle root for this FEC set
merkle_root: Hash,
}

#[derive(Clone, Copy, Debug, Eq, PartialEq, Serialize, Deserialize)]
Expand All @@ -138,6 +154,16 @@ pub(crate) struct ErasureConfig {
num_coding: usize,
}

#[cfg(test)]
impl ErasureConfig {
pub(crate) fn new(num_data: usize, num_coding: usize) -> Self {
ErasureConfig {
num_data,
num_coding,
}
}
}

#[derive(Deserialize, Serialize)]
pub struct DuplicateSlotProof {
#[serde(with = "serde_bytes")]
Expand Down Expand Up @@ -331,11 +357,14 @@ impl ErasureMeta {
num_coding: usize::from(shred.num_coding_shreds().ok()?),
};
let first_coding_index = u64::from(shred.first_coding_index()?);
let first_received_coding_index = u64::from(shred.index());
let merkle_root = Hash::default();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

next change in master will populate this from the coding shred, but I don't think that needs to be part of this backport change.

let erasure_meta = ErasureMeta {
set_index: u64::from(shred.fec_set_index()),
config,
first_coding_index,
__unused_size: 0,
first_received_coding_index,
merkle_root,
};
Some(erasure_meta)
}
Expand All @@ -348,7 +377,8 @@ impl ErasureMeta {
let Some(mut other) = Self::from_coding_shred(shred) else {
return false;
};
other.__unused_size = self.__unused_size;
// The order of received shreds should not impact consistency
other.first_received_coding_index = self.first_received_coding_index;
self == &other
}

Expand Down Expand Up @@ -394,6 +424,41 @@ impl ErasureMeta {
StillNeed(num_needed)
}
}

#[cfg(test)]
pub(crate) fn set_index(&self) -> u64 {
self.set_index
}

#[cfg(test)]
pub(crate) fn first_coding_index(&self) -> u64 {
self.first_coding_index
}

#[cfg(test)]
pub(crate) fn first_received_coding_index(&self) -> u64 {
self.first_received_coding_index
}

#[cfg(test)]
pub(crate) fn merkle_root(&self) -> Hash {
self.merkle_root
}
}

impl From<ErasureMetaLegacy> for ErasureMeta {
fn from(erasure_meta: ErasureMetaLegacy) -> ErasureMeta {
ErasureMeta {
set_index: erasure_meta.set_index,
first_coding_index: erasure_meta.first_coding_index,
// We only use this in the context of merkle_root duplicate shred
// proofs, so it's fine to not have the correct value here while
// merkle_root is Hash::default()
first_received_coding_index: erasure_meta.first_coding_index,
config: erasure_meta.config,
merkle_root: Hash::default(),
}
}
}

impl DuplicateSlotProof {
Expand Down Expand Up @@ -514,8 +579,9 @@ mod test {
let e_meta = ErasureMeta {
set_index,
first_coding_index: set_index,
first_received_coding_index: 0,
config: erasure_config,
__unused_size: 0,
merkle_root: Hash::default(),
};
let mut rng = thread_rng();
let mut index = Index::new(0);
Expand Down
1 change: 1 addition & 0 deletions ledger/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub mod block_error;
pub mod blockstore;
pub mod ancestor_iterator;
pub mod blockstore_db;
#[allow(deprecated)]
pub mod blockstore_meta;
pub mod blockstore_metrics;
pub mod blockstore_options;
Expand Down