From 5f630879ecf2d9b5f24c10faf067f03fd10c0d46 Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Wed, 29 Nov 2023 17:16:20 +0000 Subject: [PATCH 1/4] blockstore: write only dirty erasure meta and merkle root metas --- ledger/src/blockstore.rs | 66 +++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 17 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index a32af852257972..0d7724553ea896 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -732,7 +732,7 @@ impl Blockstore { fn try_shred_recovery( &self, - erasure_metas: &HashMap, + erasure_metas: &HashMap, index_working_set: &mut HashMap, prev_inserted_shreds: &HashMap, reed_solomon_cache: &ReedSolomonCache, @@ -743,7 +743,7 @@ impl Blockstore { // 2. For new data shreds, check if an erasure set exists. If not, don't try recovery // 3. Before trying recovery, check if enough number of shreds have been received // 3a. Enough number of shreds = (#data + #coding shreds) > erasure.num_data - for (erasure_set, erasure_meta) in erasure_metas.iter() { + for (erasure_set, (erasure_meta, _)) in erasure_metas.iter() { let slot = erasure_set.slot(); let index_meta_entry = index_working_set.get_mut(&slot).expect("Index"); let index = &mut index_meta_entry.index; @@ -1018,12 +1018,20 @@ impl Blockstore { &mut write_batch, )?; - for (erasure_set, erasure_meta) in erasure_metas { + for (erasure_set, (erasure_meta, is_new)) in erasure_metas { + if !is_new { + // No need to rewrite the column + continue; + } let (slot, fec_set_index) = erasure_set.store_key(); write_batch.put::((slot, u64::from(fec_set_index)), &erasure_meta)?; } - for (erasure_set, merkle_root_meta) in merkle_root_metas { + for (erasure_set, (merkle_root_meta, is_new)) in merkle_root_metas { + if !is_new { + // No need to rewrite the column + continue; + } write_batch.put::(erasure_set.store_key(), &merkle_root_meta)?; } @@ -1183,8 +1191,8 @@ impl Blockstore { fn check_insert_coding_shred( &self, shred: Shred, - erasure_metas: &mut HashMap, - merkle_root_metas: &mut HashMap, + erasure_metas: &mut HashMap, + merkle_root_metas: &mut HashMap, index_working_set: &mut HashMap, write_batch: &mut WriteBatch, just_received_shreds: &mut HashMap, @@ -1205,7 +1213,7 @@ impl Blockstore { if let HashMapEntry::Vacant(entry) = merkle_root_metas.entry(erasure_set) { if let Some(meta) = self.merkle_root_meta(erasure_set).unwrap() { - entry.insert(meta); + entry.insert((meta, /* is_new */ false)); } } @@ -1224,10 +1232,16 @@ impl Blockstore { } } - let erasure_meta = erasure_metas.entry(erasure_set).or_insert_with(|| { + let (erasure_meta, _) = erasure_metas.entry(erasure_set).or_insert_with(|| { self.erasure_meta(erasure_set) .expect("Expect database get to succeed") - .unwrap_or_else(|| ErasureMeta::from_coding_shred(&shred).unwrap()) + .map(|erasure_meta| (erasure_meta, false /* is_new */)) + .unwrap_or_else(|| { + ( + ErasureMeta::from_coding_shred(&shred).unwrap(), + true, /* is_new */ + ) + }) }); if !erasure_meta.check_coding_shred(&shred) { @@ -1289,7 +1303,7 @@ impl Blockstore { merkle_root_metas .entry(erasure_set) - .or_insert(MerkleRootMeta::from_shred(&shred)); + .or_insert((MerkleRootMeta::from_shred(&shred), true /* is _new */)); } if let HashMapEntry::Vacant(entry) = just_received_shreds.entry(shred.id()) { @@ -1372,8 +1386,8 @@ impl Blockstore { fn check_insert_data_shred( &self, shred: Shred, - erasure_metas: &mut HashMap, - merkle_root_metas: &mut HashMap, + erasure_metas: &mut HashMap, + merkle_root_metas: &mut HashMap, index_working_set: &mut HashMap, slot_meta_working_set: &mut HashMap, write_batch: &mut WriteBatch, @@ -1402,7 +1416,7 @@ impl Blockstore { let erasure_set = shred.erasure_set(); if let HashMapEntry::Vacant(entry) = merkle_root_metas.entry(erasure_set) { if let Some(meta) = self.merkle_root_meta(erasure_set).unwrap() { - entry.insert(meta); + entry.insert((meta, false /* is_new */)); } } @@ -1448,13 +1462,13 @@ impl Blockstore { )?; merkle_root_metas .entry(erasure_set) - .or_insert(MerkleRootMeta::from_shred(&shred)); + .or_insert((MerkleRootMeta::from_shred(&shred), true /* is_new */)); just_inserted_shreds.insert(shred.id(), shred); index_meta_working_set_entry.did_insert_occur = true; slot_meta_entry.did_insert_occur = true; if let HashMapEntry::Vacant(entry) = erasure_metas.entry(erasure_set) { if let Some(meta) = self.erasure_meta(erasure_set).unwrap() { - entry.insert(meta); + entry.insert((meta, false /* is_new */)); } } Ok(newly_completed_data_sets) @@ -6806,6 +6820,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() + .0 .merkle_root(), coding_shred.merkle_root().ok(), ); @@ -6813,6 +6828,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() + .0 .first_received_shred_index(), index ); @@ -6820,11 +6836,12 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() + .0 .first_received_shred_type(), ShredType::Code, ); - for (erasure_set, merkle_root_meta) in merkle_root_metas { + for (erasure_set, (merkle_root_meta, _)) in merkle_root_metas { write_batch .put::(erasure_set.store_key(), &merkle_root_meta) .unwrap(); @@ -6862,6 +6879,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() + .0 .merkle_root(), coding_shred.merkle_root().ok() ); @@ -6869,6 +6887,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() + .0 .first_received_shred_index(), index ); @@ -6918,6 +6937,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() + .0 .merkle_root(), coding_shred.merkle_root().ok() ); @@ -6925,6 +6945,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() + .0 .first_received_shred_index(), index ); @@ -6932,6 +6953,7 @@ pub mod tests { merkle_root_metas .get(&new_coding_shred.erasure_set()) .unwrap() + .0 .merkle_root(), new_coding_shred.merkle_root().ok() ); @@ -6939,6 +6961,7 @@ pub mod tests { merkle_root_metas .get(&new_coding_shred.erasure_set()) .unwrap() + .0 .first_received_shred_index(), new_index ); @@ -6986,6 +7009,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() + .0 .merkle_root(), data_shred.merkle_root().ok() ); @@ -6993,6 +7017,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() + .0 .first_received_shred_index(), index ); @@ -7000,11 +7025,12 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() + .0 .first_received_shred_type(), ShredType::Data, ); - for (erasure_set, merkle_root_meta) in merkle_root_metas { + for (erasure_set, (merkle_root_meta, _)) in merkle_root_metas { write_batch .put::(erasure_set.store_key(), &merkle_root_meta) .unwrap(); @@ -7046,6 +7072,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() + .0 .merkle_root(), data_shred.merkle_root().ok() ); @@ -7053,6 +7080,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() + .0 .first_received_shred_index(), index ); @@ -7112,6 +7140,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() + .0 .merkle_root(), data_shred.merkle_root().ok() ); @@ -7119,6 +7148,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() + .0 .first_received_shred_index(), index ); @@ -7126,6 +7156,7 @@ pub mod tests { merkle_root_metas .get(&new_data_shred.erasure_set()) .unwrap() + .0 .merkle_root(), new_data_shred.merkle_root().ok() ); @@ -7133,6 +7164,7 @@ pub mod tests { merkle_root_metas .get(&new_data_shred.erasure_set()) .unwrap() + .0 .first_received_shred_index(), new_index ); From 4c62b29ff5fe5eb4c45eb3dd0f8eecdaac6c3513 Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Thu, 30 Nov 2023 22:17:44 +0000 Subject: [PATCH 2/4] pr feedback: use enum to distinguish clean/dirty --- ledger/src/blockstore.rs | 131 ++++++++++++++++++++++++--------------- 1 file changed, 81 insertions(+), 50 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 0d7724553ea896..abe9f15221b21a 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -156,6 +156,24 @@ impl PossibleDuplicateShred { } } +enum WorkingStatus { + Dirty(T), + Clean(T), +} + +impl WorkingStatus { + fn unwrap(&self) -> &T { + match self { + Self::Dirty(value) => value, + Self::Clean(value) => value, + } + } + + fn is_dirty(&self) -> bool { + matches!(self, Self::Dirty(_)) + } +} + pub struct InsertResults { completed_data_set_infos: Vec, duplicate_shreds: Vec, @@ -732,7 +750,7 @@ impl Blockstore { fn try_shred_recovery( &self, - erasure_metas: &HashMap, + erasure_metas: &HashMap>, index_working_set: &mut HashMap, prev_inserted_shreds: &HashMap, reed_solomon_cache: &ReedSolomonCache, @@ -743,7 +761,8 @@ impl Blockstore { // 2. For new data shreds, check if an erasure set exists. If not, don't try recovery // 3. Before trying recovery, check if enough number of shreds have been received // 3a. Enough number of shreds = (#data + #coding shreds) > erasure.num_data - for (erasure_set, (erasure_meta, _)) in erasure_metas.iter() { + for (erasure_set, working_erasure_meta) in erasure_metas.iter() { + let erasure_meta = working_erasure_meta.unwrap(); let slot = erasure_set.slot(); let index_meta_entry = index_working_set.get_mut(&slot).expect("Index"); let index = &mut index_meta_entry.index; @@ -1018,21 +1037,27 @@ impl Blockstore { &mut write_batch, )?; - for (erasure_set, (erasure_meta, is_new)) in erasure_metas { - if !is_new { + for (erasure_set, working_erasure_meta) in erasure_metas { + if !working_erasure_meta.is_dirty() { // No need to rewrite the column continue; } let (slot, fec_set_index) = erasure_set.store_key(); - write_batch.put::((slot, u64::from(fec_set_index)), &erasure_meta)?; + write_batch.put::( + (slot, u64::from(fec_set_index)), + working_erasure_meta.unwrap(), + )?; } - for (erasure_set, (merkle_root_meta, is_new)) in merkle_root_metas { - if !is_new { + for (erasure_set, working_merkle_root_meta) in merkle_root_metas { + if !working_merkle_root_meta.is_dirty() { // No need to rewrite the column continue; } - write_batch.put::(erasure_set.store_key(), &merkle_root_meta)?; + write_batch.put::( + erasure_set.store_key(), + working_merkle_root_meta.unwrap(), + )?; } for (&slot, index_working_set_entry) in index_working_set.iter() { @@ -1191,8 +1216,8 @@ impl Blockstore { fn check_insert_coding_shred( &self, shred: Shred, - erasure_metas: &mut HashMap, - merkle_root_metas: &mut HashMap, + erasure_metas: &mut HashMap>, + merkle_root_metas: &mut HashMap>, index_working_set: &mut HashMap, write_batch: &mut WriteBatch, just_received_shreds: &mut HashMap, @@ -1213,7 +1238,7 @@ impl Blockstore { if let HashMapEntry::Vacant(entry) = merkle_root_metas.entry(erasure_set) { if let Some(meta) = self.merkle_root_meta(erasure_set).unwrap() { - entry.insert((meta, /* is_new */ false)); + entry.insert(WorkingStatus::Clean(meta)); } } @@ -1232,17 +1257,17 @@ impl Blockstore { } } - let (erasure_meta, _) = erasure_metas.entry(erasure_set).or_insert_with(|| { - self.erasure_meta(erasure_set) - .expect("Expect database get to succeed") - .map(|erasure_meta| (erasure_meta, false /* is_new */)) - .unwrap_or_else(|| { - ( - ErasureMeta::from_coding_shred(&shred).unwrap(), - true, /* is_new */ - ) - }) - }); + let erasure_meta = erasure_metas + .entry(erasure_set) + .or_insert_with(|| { + self.erasure_meta(erasure_set) + .expect("Expect database get to succeed") + .map(WorkingStatus::Clean) + .unwrap_or_else(|| { + WorkingStatus::Dirty(ErasureMeta::from_coding_shred(&shred).unwrap()) + }) + }) + .unwrap(); if !erasure_meta.check_coding_shred(&shred) { metrics.num_coding_shreds_invalid_erasure_config += 1; @@ -1303,7 +1328,7 @@ impl Blockstore { merkle_root_metas .entry(erasure_set) - .or_insert((MerkleRootMeta::from_shred(&shred), true /* is _new */)); + .or_insert(WorkingStatus::Dirty(MerkleRootMeta::from_shred(&shred))); } if let HashMapEntry::Vacant(entry) = just_received_shreds.entry(shred.id()) { @@ -1386,8 +1411,8 @@ impl Blockstore { fn check_insert_data_shred( &self, shred: Shred, - erasure_metas: &mut HashMap, - merkle_root_metas: &mut HashMap, + erasure_metas: &mut HashMap>, + merkle_root_metas: &mut HashMap>, index_working_set: &mut HashMap, slot_meta_working_set: &mut HashMap, write_batch: &mut WriteBatch, @@ -1416,7 +1441,7 @@ impl Blockstore { let erasure_set = shred.erasure_set(); if let HashMapEntry::Vacant(entry) = merkle_root_metas.entry(erasure_set) { if let Some(meta) = self.merkle_root_meta(erasure_set).unwrap() { - entry.insert((meta, false /* is_new */)); + entry.insert(WorkingStatus::Clean(meta)); } } @@ -1462,13 +1487,13 @@ impl Blockstore { )?; merkle_root_metas .entry(erasure_set) - .or_insert((MerkleRootMeta::from_shred(&shred), true /* is_new */)); + .or_insert(WorkingStatus::Dirty(MerkleRootMeta::from_shred(&shred))); just_inserted_shreds.insert(shred.id(), shred); index_meta_working_set_entry.did_insert_occur = true; slot_meta_entry.did_insert_occur = true; if let HashMapEntry::Vacant(entry) = erasure_metas.entry(erasure_set) { if let Some(meta) = self.erasure_meta(erasure_set).unwrap() { - entry.insert((meta, false /* is_new */)); + entry.insert(WorkingStatus::Clean(meta)); } } Ok(newly_completed_data_sets) @@ -6820,7 +6845,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .merkle_root(), coding_shred.merkle_root().ok(), ); @@ -6828,7 +6853,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_index(), index ); @@ -6836,14 +6861,17 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_type(), ShredType::Code, ); - for (erasure_set, (merkle_root_meta, _)) in merkle_root_metas { + for (erasure_set, working_merkle_root_meta) in merkle_root_metas { write_batch - .put::(erasure_set.store_key(), &merkle_root_meta) + .put::( + erasure_set.store_key(), + working_merkle_root_meta.unwrap(), + ) .unwrap(); } blockstore.db.write(write_batch).unwrap(); @@ -6879,7 +6907,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .merkle_root(), coding_shred.merkle_root().ok() ); @@ -6887,7 +6915,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_index(), index ); @@ -6937,7 +6965,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .merkle_root(), coding_shred.merkle_root().ok() ); @@ -6945,7 +6973,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_index(), index ); @@ -6953,7 +6981,7 @@ pub mod tests { merkle_root_metas .get(&new_coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .merkle_root(), new_coding_shred.merkle_root().ok() ); @@ -6961,7 +6989,7 @@ pub mod tests { merkle_root_metas .get(&new_coding_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_index(), new_index ); @@ -7009,7 +7037,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7017,7 +7045,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_index(), index ); @@ -7025,14 +7053,17 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_type(), ShredType::Data, ); - for (erasure_set, (merkle_root_meta, _)) in merkle_root_metas { + for (erasure_set, working_merkle_root_meta) in merkle_root_metas { write_batch - .put::(erasure_set.store_key(), &merkle_root_meta) + .put::( + erasure_set.store_key(), + working_merkle_root_meta.unwrap(), + ) .unwrap(); } blockstore.db.write(write_batch).unwrap(); @@ -7072,7 +7103,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7080,7 +7111,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_index(), index ); @@ -7140,7 +7171,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7148,7 +7179,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_index(), index ); @@ -7156,7 +7187,7 @@ pub mod tests { merkle_root_metas .get(&new_data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .merkle_root(), new_data_shred.merkle_root().ok() ); @@ -7164,7 +7195,7 @@ pub mod tests { merkle_root_metas .get(&new_data_shred.erasure_set()) .unwrap() - .0 + .unwrap() .first_received_shred_index(), new_index ); From 1bec3454133f8d83a1a3283d667e90907ad6b9ee Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Tue, 12 Dec 2023 21:53:04 +0000 Subject: [PATCH 3/4] pr feedback: comments, rename --- ledger/src/blockstore.rs | 89 ++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 44 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index abe9f15221b21a..55b443afbd0e7c 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -156,20 +156,21 @@ impl PossibleDuplicateShred { } } -enum WorkingStatus { - Dirty(T), - Clean(T), +enum WorkingEntry { + Dirty(T), // Value has been modified with respect to the blockstore column + Clean(T), // Value matches what is currently in the blockstore column } -impl WorkingStatus { - fn unwrap(&self) -> &T { +impl WorkingEntry { + /// Returns a reference to the underlying value + fn value(&self) -> &T { match self { Self::Dirty(value) => value, Self::Clean(value) => value, } } - fn is_dirty(&self) -> bool { + fn should_write(&self) -> bool { matches!(self, Self::Dirty(_)) } } @@ -750,7 +751,7 @@ impl Blockstore { fn try_shred_recovery( &self, - erasure_metas: &HashMap>, + erasure_metas: &HashMap>, index_working_set: &mut HashMap, prev_inserted_shreds: &HashMap, reed_solomon_cache: &ReedSolomonCache, @@ -762,7 +763,7 @@ impl Blockstore { // 3. Before trying recovery, check if enough number of shreds have been received // 3a. Enough number of shreds = (#data + #coding shreds) > erasure.num_data for (erasure_set, working_erasure_meta) in erasure_metas.iter() { - let erasure_meta = working_erasure_meta.unwrap(); + let erasure_meta = working_erasure_meta.value(); let slot = erasure_set.slot(); let index_meta_entry = index_working_set.get_mut(&slot).expect("Index"); let index = &mut index_meta_entry.index; @@ -1038,25 +1039,25 @@ impl Blockstore { )?; for (erasure_set, working_erasure_meta) in erasure_metas { - if !working_erasure_meta.is_dirty() { + if !working_erasure_meta.should_write() { // No need to rewrite the column continue; } let (slot, fec_set_index) = erasure_set.store_key(); write_batch.put::( (slot, u64::from(fec_set_index)), - working_erasure_meta.unwrap(), + working_erasure_meta.value(), )?; } for (erasure_set, working_merkle_root_meta) in merkle_root_metas { - if !working_merkle_root_meta.is_dirty() { + if !working_merkle_root_meta.should_write() { // No need to rewrite the column continue; } write_batch.put::( erasure_set.store_key(), - working_merkle_root_meta.unwrap(), + working_merkle_root_meta.value(), )?; } @@ -1216,8 +1217,8 @@ impl Blockstore { fn check_insert_coding_shred( &self, shred: Shred, - erasure_metas: &mut HashMap>, - merkle_root_metas: &mut HashMap>, + erasure_metas: &mut HashMap>, + merkle_root_metas: &mut HashMap>, index_working_set: &mut HashMap, write_batch: &mut WriteBatch, just_received_shreds: &mut HashMap, @@ -1238,7 +1239,7 @@ impl Blockstore { if let HashMapEntry::Vacant(entry) = merkle_root_metas.entry(erasure_set) { if let Some(meta) = self.merkle_root_meta(erasure_set).unwrap() { - entry.insert(WorkingStatus::Clean(meta)); + entry.insert(WorkingEntry::Clean(meta)); } } @@ -1262,12 +1263,12 @@ impl Blockstore { .or_insert_with(|| { self.erasure_meta(erasure_set) .expect("Expect database get to succeed") - .map(WorkingStatus::Clean) + .map(WorkingEntry::Clean) .unwrap_or_else(|| { - WorkingStatus::Dirty(ErasureMeta::from_coding_shred(&shred).unwrap()) + WorkingEntry::Dirty(ErasureMeta::from_coding_shred(&shred).unwrap()) }) }) - .unwrap(); + .value(); if !erasure_meta.check_coding_shred(&shred) { metrics.num_coding_shreds_invalid_erasure_config += 1; @@ -1328,7 +1329,7 @@ impl Blockstore { merkle_root_metas .entry(erasure_set) - .or_insert(WorkingStatus::Dirty(MerkleRootMeta::from_shred(&shred))); + .or_insert(WorkingEntry::Dirty(MerkleRootMeta::from_shred(&shred))); } if let HashMapEntry::Vacant(entry) = just_received_shreds.entry(shred.id()) { @@ -1411,8 +1412,8 @@ impl Blockstore { fn check_insert_data_shred( &self, shred: Shred, - erasure_metas: &mut HashMap>, - merkle_root_metas: &mut HashMap>, + erasure_metas: &mut HashMap>, + merkle_root_metas: &mut HashMap>, index_working_set: &mut HashMap, slot_meta_working_set: &mut HashMap, write_batch: &mut WriteBatch, @@ -1441,7 +1442,7 @@ impl Blockstore { let erasure_set = shred.erasure_set(); if let HashMapEntry::Vacant(entry) = merkle_root_metas.entry(erasure_set) { if let Some(meta) = self.merkle_root_meta(erasure_set).unwrap() { - entry.insert(WorkingStatus::Clean(meta)); + entry.insert(WorkingEntry::Clean(meta)); } } @@ -1487,13 +1488,13 @@ impl Blockstore { )?; merkle_root_metas .entry(erasure_set) - .or_insert(WorkingStatus::Dirty(MerkleRootMeta::from_shred(&shred))); + .or_insert(WorkingEntry::Dirty(MerkleRootMeta::from_shred(&shred))); just_inserted_shreds.insert(shred.id(), shred); index_meta_working_set_entry.did_insert_occur = true; slot_meta_entry.did_insert_occur = true; if let HashMapEntry::Vacant(entry) = erasure_metas.entry(erasure_set) { if let Some(meta) = self.erasure_meta(erasure_set).unwrap() { - entry.insert(WorkingStatus::Clean(meta)); + entry.insert(WorkingEntry::Clean(meta)); } } Ok(newly_completed_data_sets) @@ -6845,7 +6846,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .merkle_root(), coding_shred.merkle_root().ok(), ); @@ -6853,7 +6854,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_index(), index ); @@ -6861,7 +6862,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_type(), ShredType::Code, ); @@ -6870,7 +6871,7 @@ pub mod tests { write_batch .put::( erasure_set.store_key(), - working_merkle_root_meta.unwrap(), + working_merkle_root_meta.value(), ) .unwrap(); } @@ -6907,7 +6908,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .merkle_root(), coding_shred.merkle_root().ok() ); @@ -6915,7 +6916,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_index(), index ); @@ -6965,7 +6966,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .merkle_root(), coding_shred.merkle_root().ok() ); @@ -6973,7 +6974,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_index(), index ); @@ -6981,7 +6982,7 @@ pub mod tests { merkle_root_metas .get(&new_coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .merkle_root(), new_coding_shred.merkle_root().ok() ); @@ -6989,7 +6990,7 @@ pub mod tests { merkle_root_metas .get(&new_coding_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_index(), new_index ); @@ -7037,7 +7038,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7045,7 +7046,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_index(), index ); @@ -7053,7 +7054,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_type(), ShredType::Data, ); @@ -7062,7 +7063,7 @@ pub mod tests { write_batch .put::( erasure_set.store_key(), - working_merkle_root_meta.unwrap(), + working_merkle_root_meta.value(), ) .unwrap(); } @@ -7103,7 +7104,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7111,7 +7112,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_index(), index ); @@ -7171,7 +7172,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7179,7 +7180,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_index(), index ); @@ -7187,7 +7188,7 @@ pub mod tests { merkle_root_metas .get(&new_data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .merkle_root(), new_data_shred.merkle_root().ok() ); @@ -7195,7 +7196,7 @@ pub mod tests { merkle_root_metas .get(&new_data_shred.erasure_set()) .unwrap() - .unwrap() + .value() .first_received_shred_index(), new_index ); From bf107c9013935aeca9c9e668e290dff1557c975c Mon Sep 17 00:00:00 2001 From: Ashwin Sekar Date: Fri, 22 Dec 2023 18:09:25 +0000 Subject: [PATCH 4/4] pr feedback: use AsRef --- ledger/src/blockstore.rs | 79 ++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 40 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 55b443afbd0e7c..591034aab894c0 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -162,17 +162,18 @@ enum WorkingEntry { } impl WorkingEntry { - /// Returns a reference to the underlying value - fn value(&self) -> &T { + fn should_write(&self) -> bool { + matches!(self, Self::Dirty(_)) + } +} + +impl AsRef for WorkingEntry { + fn as_ref(&self) -> &T { match self { Self::Dirty(value) => value, Self::Clean(value) => value, } } - - fn should_write(&self) -> bool { - matches!(self, Self::Dirty(_)) - } } pub struct InsertResults { @@ -763,7 +764,7 @@ impl Blockstore { // 3. Before trying recovery, check if enough number of shreds have been received // 3a. Enough number of shreds = (#data + #coding shreds) > erasure.num_data for (erasure_set, working_erasure_meta) in erasure_metas.iter() { - let erasure_meta = working_erasure_meta.value(); + let erasure_meta = working_erasure_meta.as_ref(); let slot = erasure_set.slot(); let index_meta_entry = index_working_set.get_mut(&slot).expect("Index"); let index = &mut index_meta_entry.index; @@ -1046,7 +1047,7 @@ impl Blockstore { let (slot, fec_set_index) = erasure_set.store_key(); write_batch.put::( (slot, u64::from(fec_set_index)), - working_erasure_meta.value(), + working_erasure_meta.as_ref(), )?; } @@ -1057,7 +1058,7 @@ impl Blockstore { } write_batch.put::( erasure_set.store_key(), - working_merkle_root_meta.value(), + working_merkle_root_meta.as_ref(), )?; } @@ -1258,17 +1259,15 @@ impl Blockstore { } } - let erasure_meta = erasure_metas - .entry(erasure_set) - .or_insert_with(|| { - self.erasure_meta(erasure_set) - .expect("Expect database get to succeed") - .map(WorkingEntry::Clean) - .unwrap_or_else(|| { - WorkingEntry::Dirty(ErasureMeta::from_coding_shred(&shred).unwrap()) - }) - }) - .value(); + let erasure_meta_entry = erasure_metas.entry(erasure_set).or_insert_with(|| { + self.erasure_meta(erasure_set) + .expect("Expect database get to succeed") + .map(WorkingEntry::Clean) + .unwrap_or_else(|| { + WorkingEntry::Dirty(ErasureMeta::from_coding_shred(&shred).unwrap()) + }) + }); + let erasure_meta = erasure_meta_entry.as_ref(); if !erasure_meta.check_coding_shred(&shred) { metrics.num_coding_shreds_invalid_erasure_config += 1; @@ -6846,7 +6845,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .merkle_root(), coding_shred.merkle_root().ok(), ); @@ -6854,7 +6853,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_index(), index ); @@ -6862,7 +6861,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_type(), ShredType::Code, ); @@ -6871,7 +6870,7 @@ pub mod tests { write_batch .put::( erasure_set.store_key(), - working_merkle_root_meta.value(), + working_merkle_root_meta.as_ref(), ) .unwrap(); } @@ -6908,7 +6907,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .merkle_root(), coding_shred.merkle_root().ok() ); @@ -6916,7 +6915,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_index(), index ); @@ -6966,7 +6965,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .merkle_root(), coding_shred.merkle_root().ok() ); @@ -6974,7 +6973,7 @@ pub mod tests { merkle_root_metas .get(&coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_index(), index ); @@ -6982,7 +6981,7 @@ pub mod tests { merkle_root_metas .get(&new_coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .merkle_root(), new_coding_shred.merkle_root().ok() ); @@ -6990,7 +6989,7 @@ pub mod tests { merkle_root_metas .get(&new_coding_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_index(), new_index ); @@ -7038,7 +7037,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7046,7 +7045,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_index(), index ); @@ -7054,7 +7053,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_type(), ShredType::Data, ); @@ -7063,7 +7062,7 @@ pub mod tests { write_batch .put::( erasure_set.store_key(), - working_merkle_root_meta.value(), + working_merkle_root_meta.as_ref(), ) .unwrap(); } @@ -7104,7 +7103,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7112,7 +7111,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_index(), index ); @@ -7172,7 +7171,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .merkle_root(), data_shred.merkle_root().ok() ); @@ -7180,7 +7179,7 @@ pub mod tests { merkle_root_metas .get(&data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_index(), index ); @@ -7188,7 +7187,7 @@ pub mod tests { merkle_root_metas .get(&new_data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .merkle_root(), new_data_shred.merkle_root().ok() ); @@ -7196,7 +7195,7 @@ pub mod tests { merkle_root_metas .get(&new_data_shred.erasure_set()) .unwrap() - .value() + .as_ref() .first_received_shred_index(), new_index );