From eba10894b39f489921e5fa5dca4a8302d62c0630 Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Thu, 9 Nov 2023 17:02:15 +0000 Subject: [PATCH] pr feedback: remove write/reads to column --- ledger/src/blockstore.rs | 429 ---------------------------------- ledger/src/blockstore_db.rs | 14 +- ledger/src/blockstore_meta.rs | 25 -- ledger/src/shred.rs | 4 - 4 files changed, 7 insertions(+), 465 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index dcc2ceafa11cac..b1669919273ae5 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -845,7 +845,6 @@ impl Blockstore { let mut just_inserted_shreds = HashMap::with_capacity(shreds.len()); let mut erasure_metas = HashMap::new(); - let mut merkle_root_metas = HashMap::new(); let mut slot_meta_working_set = HashMap::new(); let mut index_working_set = HashMap::new(); let mut duplicate_shreds = vec![]; @@ -865,7 +864,6 @@ impl Blockstore { match self.check_insert_data_shred( shred, &mut erasure_metas, - &mut merkle_root_metas, &mut index_working_set, &mut slot_meta_working_set, &mut write_batch, @@ -903,7 +901,6 @@ impl Blockstore { self.check_insert_coding_shred( shred, &mut erasure_metas, - &mut merkle_root_metas, &mut index_working_set, &mut write_batch, &mut just_inserted_shreds, @@ -950,7 +947,6 @@ impl Blockstore { match self.check_insert_data_shred( shred.clone(), &mut erasure_metas, - &mut merkle_root_metas, &mut index_working_set, &mut slot_meta_working_set, &mut write_batch, @@ -1016,10 +1012,6 @@ impl Blockstore { write_batch.put::(erasure_set.store_key(), &erasure_meta)?; } - for (erasure_set, merkle_root_meta) in merkle_root_metas { - write_batch.put::(erasure_set.store_key(), &merkle_root_meta)?; - } - for (&slot, index_working_set_entry) in index_working_set.iter() { if index_working_set_entry.did_insert_occur { write_batch.put::(slot, &index_working_set_entry.index)?; @@ -1177,7 +1169,6 @@ impl Blockstore { &self, shred: Shred, erasure_metas: &mut HashMap, - merkle_root_metas: &mut HashMap, index_working_set: &mut HashMap, write_batch: &mut WriteBatch, just_received_shreds: &mut HashMap, @@ -1217,11 +1208,6 @@ impl Blockstore { .expect("Expect database get to succeed") .unwrap_or_else(|| ErasureMeta::from_coding_shred(&shred).unwrap()) }); - let _merkle_root_meta = merkle_root_metas.entry(erasure_set).or_insert_with(|| { - self.merkle_root_meta(erasure_set) - .expect("Expect database get to succeed") - .unwrap_or_else(|| MerkleRootMeta::from_shred(&shred)) - }); if !erasure_meta.check_coding_shred(&shred) { metrics.num_coding_shreds_invalid_erasure_config += 1; @@ -1359,7 +1345,6 @@ impl Blockstore { &self, shred: Shred, erasure_metas: &mut HashMap, - merkle_root_metas: &mut HashMap, index_working_set: &mut HashMap, slot_meta_working_set: &mut HashMap, write_batch: &mut WriteBatch, @@ -1386,11 +1371,6 @@ impl Blockstore { let slot_meta = &mut slot_meta_entry.new_slot_meta.borrow_mut(); let erasure_set = shred.erasure_set(); - let _merkle_root_meta = merkle_root_metas.entry(erasure_set).or_insert_with(|| { - self.merkle_root_meta(erasure_set) - .expect("Expect database get to succeed") - .unwrap_or_else(|| MerkleRootMeta::from_shred(&shred)) - }); if !is_trusted { if Self::is_data_shred_present(&shred, slot_meta, index_meta.data()) { @@ -3218,10 +3198,6 @@ impl Blockstore { .unwrap_or(false) } - fn merkle_root_meta(&self, erasure_set: ErasureSetId) -> Result> { - self.merkle_root_meta_cf.get(erasure_set.store_key()) - } - pub fn insert_optimistic_slot( &self, slot: Slot, @@ -6758,408 +6734,6 @@ pub mod tests { ),); } - #[test] - fn test_merkle_root_metas_coding() { - let ledger_path = get_tmp_ledger_path_auto_delete!(); - let blockstore = Blockstore::open(ledger_path.path()).unwrap(); - - let slot = 1; - let index = 12; - let fec_set_index = 11; - let coding_shred = Shred::new_from_parity_shard( - slot, - index, - &[], // parity_shard - fec_set_index, - 11, // num_data_shreds - 11, // num_coding_shreds - 8, // position - 0, // version - ); - - let mut erasure_metas = HashMap::new(); - let mut merkle_root_metas = HashMap::new(); - let mut index_working_set = HashMap::new(); - let mut just_received_shreds = HashMap::new(); - let mut write_batch = blockstore.db.batch().unwrap(); - let mut index_meta_time_us = 0; - assert!(blockstore.check_insert_coding_shred( - coding_shred.clone(), - &mut erasure_metas, - &mut merkle_root_metas, - &mut index_working_set, - &mut write_batch, - &mut just_received_shreds, - &mut index_meta_time_us, - &mut vec![], - false, - ShredSource::Turbine, - &mut BlockstoreInsertionMetrics::default(), - )); - - assert_eq!(merkle_root_metas.len(), 1); - assert_eq!( - merkle_root_metas - .get(&coding_shred.erasure_set()) - .unwrap() - .merkle_root(), - coding_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - merkle_root_metas - .get(&coding_shred.erasure_set()) - .unwrap() - .first_received_shred_index(), - index as u64 - ); - assert_eq!( - merkle_root_metas - .get(&coding_shred.erasure_set()) - .unwrap() - .first_received_shred_type(), - ShredType::Code, - ); - - for (erasure_set, merkle_root_meta) in merkle_root_metas { - write_batch - .put::(erasure_set.store_key(), &merkle_root_meta) - .unwrap(); - } - blockstore.db.write(write_batch).unwrap(); - - // Add a shred with different merkle root and index - let new_coding_shred = Shred::new_from_parity_shard( - slot, - index + 1, - &[], // parity_shard - fec_set_index, - 11, // num_data_shreds - 11, // num_coding_shreds - 8, // position - 0, // version - ); - - erasure_metas.clear(); - index_working_set.clear(); - just_received_shreds.clear(); - let mut merkle_root_metas = HashMap::new(); - let mut write_batch = blockstore.db.batch().unwrap(); - - assert!(blockstore.check_insert_coding_shred( - new_coding_shred.clone(), - &mut erasure_metas, - &mut merkle_root_metas, - &mut index_working_set, - &mut write_batch, - &mut just_received_shreds, - &mut index_meta_time_us, - &mut vec![], - false, - ShredSource::Turbine, - &mut BlockstoreInsertionMetrics::default(), - )); - - // Verify that we still have the merkle root meta from the original shred - assert_eq!(merkle_root_metas.len(), 1); - assert_eq!( - merkle_root_metas - .get(&coding_shred.erasure_set()) - .unwrap() - .merkle_root(), - coding_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - merkle_root_metas - .get(&coding_shred.erasure_set()) - .unwrap() - .first_received_shred_index(), - index as u64 - ); - - // Blockstore should also have the merkle root meta of the original shred - assert_eq!( - blockstore - .merkle_root_meta(coding_shred.erasure_set()) - .unwrap() - .unwrap() - .merkle_root(), - coding_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - blockstore - .merkle_root_meta(coding_shred.erasure_set()) - .unwrap() - .unwrap() - .first_received_shred_index(), - index as u64 - ); - - // Add a shred from different fec set - let new_index = fec_set_index + 31; - let new_coding_shred = Shred::new_from_parity_shard( - slot, - new_index, - &[], // parity_shard - fec_set_index + 30, // fec_set_index - 11, // num_data_shreds - 11, // num_coding_shreds - 8, // position - 0, // version - ); - - assert!(blockstore.check_insert_coding_shred( - new_coding_shred.clone(), - &mut erasure_metas, - &mut merkle_root_metas, - &mut index_working_set, - &mut write_batch, - &mut just_received_shreds, - &mut index_meta_time_us, - &mut vec![], - false, - ShredSource::Turbine, - &mut BlockstoreInsertionMetrics::default(), - )); - - // Verify that we still have the merkle root meta for the original shred - // and the new shred - assert_eq!(merkle_root_metas.len(), 2); - assert_eq!( - merkle_root_metas - .get(&coding_shred.erasure_set()) - .unwrap() - .merkle_root(), - coding_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - merkle_root_metas - .get(&coding_shred.erasure_set()) - .unwrap() - .first_received_shred_index(), - index as u64 - ); - assert_eq!( - merkle_root_metas - .get(&new_coding_shred.erasure_set()) - .unwrap() - .merkle_root(), - new_coding_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - merkle_root_metas - .get(&new_coding_shred.erasure_set()) - .unwrap() - .first_received_shred_index(), - new_index as u64 - ); - } - - #[test] - fn test_merkle_root_metas_data() { - let ledger_path = get_tmp_ledger_path_auto_delete!(); - let blockstore = Blockstore::open(ledger_path.path()).unwrap(); - - let slot = 1; - let index = 12; - let fec_set_index = 11; - let data_shred = Shred::new_from_data( - slot, - index, - 1, // parent_offset - &[1, 1, 1], // data - ShredFlags::empty(), - 0, // reference_tick, - 0, // version - fec_set_index, - ); - - let mut erasure_metas = HashMap::new(); - let mut merkle_root_metas = HashMap::new(); - let mut index_working_set = HashMap::new(); - let mut just_received_shreds = HashMap::new(); - let mut slot_meta_working_set = HashMap::new(); - let mut write_batch = blockstore.db.batch().unwrap(); - let mut index_meta_time_us = 0; - blockstore - .check_insert_data_shred( - data_shred.clone(), - &mut erasure_metas, - &mut merkle_root_metas, - &mut index_working_set, - &mut slot_meta_working_set, - &mut write_batch, - &mut just_received_shreds, - &mut index_meta_time_us, - false, - &mut vec![], - None, - ShredSource::Turbine, - ) - .unwrap(); - - assert_eq!(merkle_root_metas.len(), 1); - assert_eq!( - merkle_root_metas - .get(&data_shred.erasure_set()) - .unwrap() - .merkle_root(), - data_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - merkle_root_metas - .get(&data_shred.erasure_set()) - .unwrap() - .first_received_shred_index(), - index as u64 - ); - assert_eq!( - merkle_root_metas - .get(&data_shred.erasure_set()) - .unwrap() - .first_received_shred_type(), - ShredType::Data, - ); - - for (erasure_set, merkle_root_meta) in merkle_root_metas { - write_batch - .put::(erasure_set.store_key(), &merkle_root_meta) - .unwrap(); - } - blockstore.db.write(write_batch).unwrap(); - - // Add a shred with different merkle root and index - let new_data_shred = Shred::new_from_data( - slot, - index + 1, - 1, // parent_offset - &[2, 2, 2], // data - ShredFlags::empty(), - 0, // reference_tick, - 0, // version - fec_set_index, - ); - - erasure_metas.clear(); - index_working_set.clear(); - just_received_shreds.clear(); - let mut merkle_root_metas = HashMap::new(); - let mut write_batch = blockstore.db.batch().unwrap(); - - blockstore - .check_insert_data_shred( - new_data_shred.clone(), - &mut erasure_metas, - &mut merkle_root_metas, - &mut index_working_set, - &mut slot_meta_working_set, - &mut write_batch, - &mut just_received_shreds, - &mut index_meta_time_us, - false, - &mut vec![], - None, - ShredSource::Turbine, - ) - .unwrap(); - - // Verify that we still have the merkle root meta from the original shred - assert_eq!(merkle_root_metas.len(), 1); - assert_eq!( - merkle_root_metas - .get(&data_shred.erasure_set()) - .unwrap() - .merkle_root(), - data_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - merkle_root_metas - .get(&data_shred.erasure_set()) - .unwrap() - .first_received_shred_index(), - index as u64 - ); - - // Blockstore should also have the merkle root meta of the original shred - assert_eq!( - blockstore - .merkle_root_meta(data_shred.erasure_set()) - .unwrap() - .unwrap() - .merkle_root(), - data_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - blockstore - .merkle_root_meta(data_shred.erasure_set()) - .unwrap() - .unwrap() - .first_received_shred_index(), - index as u64 - ); - - // Add a shred from different fec set - let new_index = fec_set_index + 31; - let new_data_shred = Shred::new_from_data( - slot, - new_index, - 1, // parent_offset - &[3, 3, 3], // data - ShredFlags::empty(), - 0, // reference_tick, - 0, // version - fec_set_index + 30, - ); - - blockstore - .check_insert_data_shred( - new_data_shred.clone(), - &mut erasure_metas, - &mut merkle_root_metas, - &mut index_working_set, - &mut slot_meta_working_set, - &mut write_batch, - &mut just_received_shreds, - &mut index_meta_time_us, - false, - &mut vec![], - None, - ShredSource::Turbine, - ) - .unwrap(); - - // Verify that we still have the merkle root meta for the original shred - // and the new shred - assert_eq!(merkle_root_metas.len(), 2); - assert_eq!( - merkle_root_metas - .get(&data_shred.erasure_set()) - .unwrap() - .merkle_root(), - data_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - merkle_root_metas - .get(&data_shred.erasure_set()) - .unwrap() - .first_received_shred_index(), - index as u64 - ); - assert_eq!( - merkle_root_metas - .get(&new_data_shred.erasure_set()) - .unwrap() - .merkle_root(), - new_data_shred.merkle_root().unwrap_or_default() - ); - assert_eq!( - merkle_root_metas - .get(&new_data_shred.erasure_set()) - .unwrap() - .first_received_shred_index(), - new_index as u64 - ); - } - #[test] fn test_check_insert_coding_shred() { let ledger_path = get_tmp_ledger_path_auto_delete!(); @@ -7178,7 +6752,6 @@ pub mod tests { ); let mut erasure_metas = HashMap::new(); - let mut merkle_root_metas = HashMap::new(); let mut index_working_set = HashMap::new(); let mut just_received_shreds = HashMap::new(); let mut write_batch = blockstore.db.batch().unwrap(); @@ -7186,7 +6759,6 @@ pub mod tests { assert!(blockstore.check_insert_coding_shred( coding_shred.clone(), &mut erasure_metas, - &mut merkle_root_metas, &mut index_working_set, &mut write_batch, &mut just_received_shreds, @@ -7202,7 +6774,6 @@ pub mod tests { assert!(!blockstore.check_insert_coding_shred( coding_shred.clone(), &mut erasure_metas, - &mut merkle_root_metas, &mut index_working_set, &mut write_batch, &mut just_received_shreds, diff --git a/ledger/src/blockstore_db.rs b/ledger/src/blockstore_db.rs index 627e35e88ec82e..408e53b0837c01 100644 --- a/ledger/src/blockstore_db.rs +++ b/ledger/src/blockstore_db.rs @@ -105,7 +105,7 @@ const PROGRAM_COSTS_CF: &str = "program_costs"; /// Column family for optimistic slots const OPTIMISTIC_SLOTS_CF: &str = "optimistic_slots"; /// Column family for merkle roots -const MERKLE_ROOT_CF: &str = "merkle_root_meta"; +const MERKLE_ROOT_META_CF: &str = "merkle_root_meta"; #[derive(Error, Debug)] pub enum BlockstoreError { @@ -349,7 +349,7 @@ pub mod columns { /// its FEC set. This column stores that merkle root and associated /// meta information about the first shred received. /// - /// Its index type is (Slot, FEC) set index. + /// Its index type is (Slot, fec_set_index). /// /// * index type: `crate::shred::ErasureSetId` `(Slot, fec_set_index: u64)` /// * value type: [`blockstore_meta::MerkleRootMeta`]` @@ -1250,15 +1250,15 @@ impl Column for columns::MerkleRootMeta { fn index(key: &[u8]) -> (Slot, u64) { let slot = BigEndian::read_u64(&key[..8]); - let set_index = BigEndian::read_u64(&key[8..]); + let fec_set_index = BigEndian::read_u64(&key[8..]); - (slot, set_index) + (slot, fec_set_index) } - fn key((slot, set_index): (Slot, u64)) -> Vec { + fn key((slot, fec_set_index): (Slot, u64)) -> Vec { let mut key = vec![0; 16]; BigEndian::write_u64(&mut key[..8], slot); - BigEndian::write_u64(&mut key[8..], set_index); + BigEndian::write_u64(&mut key[8..], fec_set_index); key } @@ -1272,7 +1272,7 @@ impl Column for columns::MerkleRootMeta { } impl ColumnName for columns::MerkleRootMeta { - const NAME: &'static str = MERKLE_ROOT_CF; + const NAME: &'static str = MERKLE_ROOT_META_CF; } impl TypedColumn for columns::MerkleRootMeta { type Type = MerkleRootMeta; diff --git a/ledger/src/blockstore_meta.rs b/ledger/src/blockstore_meta.rs index f69f70d1ce215a..77595a870ca7e1 100644 --- a/ledger/src/blockstore_meta.rs +++ b/ledger/src/blockstore_meta.rs @@ -406,31 +406,6 @@ impl ErasureMeta { } } -impl MerkleRootMeta { - pub(crate) fn from_shred(shred: &Shred) -> Self { - Self { - merkle_root: shred.merkle_root().unwrap_or_default(), - first_received_shred_index: u64::from(shred.index()), - first_received_shred_type: shred.shred_type(), - } - } - - #[cfg(test)] - pub(crate) fn merkle_root(&self) -> Hash { - self.merkle_root - } - - #[cfg(test)] - pub(crate) fn first_received_shred_index(&self) -> u64 { - self.first_received_shred_index - } - - #[cfg(test)] - pub(crate) fn first_received_shred_type(&self) -> ShredType { - self.first_received_shred_type - } -} - impl DuplicateSlotProof { pub(crate) fn new(shred1: Vec, shred2: Vec) -> Self { DuplicateSlotProof { shred1, shred2 } diff --git a/ledger/src/shred.rs b/ledger/src/shred.rs index 50056cd3ab1e33..5fda160e29b976 100644 --- a/ledger/src/shred.rs +++ b/ledger/src/shred.rs @@ -551,10 +551,6 @@ impl Shred { Self::ShredData(_) => Err(Error::InvalidShredType), } } - - pub fn merkle_root(&self) -> Option { - layout::get_merkle_root(self.payload()) - } } // Helper methods to extract pieces of the shred from the payload