From dc69ddce76aabd9d028235632c23246fa311095a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Chabowski?= Date: Mon, 2 Oct 2023 11:26:07 +0200 Subject: [PATCH 1/3] Short-circuit initialization of block and deploy metadata DB --- node/src/components/storage.rs | 45 +++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 17 deletions(-) diff --git a/node/src/components/storage.rs b/node/src/components/storage.rs index 8a08496800..05ee8c6fa9 100644 --- a/node/src/components/storage.rs +++ b/node/src/components/storage.rs @@ -2981,21 +2981,27 @@ fn initialize_block_metadata_db( block_metadata_db: &Database, deleted_block_hashes: &HashSet<&[u8]>, ) -> Result<(), FatalStorageError> { - info!("initializing block metadata database"); - let mut txn = env.begin_rw_txn()?; - let mut cursor = txn.open_rw_cursor(*block_metadata_db)?; + let block_count_to_be_deleted = deleted_block_hashes.len(); + info!( + block_count_to_be_deleted, + "initializing block metadata database" + ); - for row in cursor.iter() { - let (raw_key, _) = row?; - if deleted_block_hashes.contains(raw_key) { - cursor.del(WriteFlags::empty())?; - continue; + if !deleted_block_hashes.is_empty() { + let mut txn = env.begin_rw_txn()?; + let mut cursor = txn.open_rw_cursor(*block_metadata_db)?; + + for row in cursor.iter() { + let (raw_key, _) = row?; + if deleted_block_hashes.contains(raw_key) { + cursor.del(WriteFlags::empty())?; + continue; + } } + drop(cursor); + txn.commit()?; } - drop(cursor); - txn.commit()?; - info!("block metadata database initialized"); Ok(()) } @@ -3006,15 +3012,20 @@ fn initialize_deploy_metadata_db( deploy_metadata_db: &Database, deleted_deploy_hashes: &HashSet, ) -> Result<(), LmdbExtError> { - info!("initializing deploy metadata database"); + let deploy_count_to_be_deleted = deleted_deploy_hashes.len(); + info!( + deploy_count_to_be_deleted, + "initializing deploy metadata database" + ); - let mut txn = env.begin_rw_txn()?; - deleted_deploy_hashes.iter().for_each(|deleted_deploy_hash| { + if !deleted_deploy_hashes.is_empty() { + let mut txn = env.begin_rw_txn()?; + deleted_deploy_hashes.iter().for_each(|deleted_deploy_hash| { if txn.del(*deploy_metadata_db, deleted_deploy_hash, None).is_err() { debug!(%deleted_deploy_hash, "not purging from 'deploy_metadata_db' because not existing"); - } - }); - txn.commit()?; + }}); + txn.commit()?; + } info!("deploy metadata database initialized"); Ok(()) From 37c7534b8b380c9209e3dbc0bede46477bddd9e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Chabowski?= Date: Tue, 3 Oct 2023 17:10:49 +0200 Subject: [PATCH 2/3] Add test for `initialize_block_metadata_db` --- node/src/components/storage.rs | 39 ++++---- node/src/components/storage/tests.rs | 127 ++++++++++++++++++++++++++- 2 files changed, 145 insertions(+), 21 deletions(-) diff --git a/node/src/components/storage.rs b/node/src/components/storage.rs index 05ee8c6fa9..12eb37847f 100644 --- a/node/src/components/storage.rs +++ b/node/src/components/storage.rs @@ -1118,23 +1118,9 @@ impl Storage { StorageRequest::PutFinalitySignature { signature, responder, - } => { - let mut txn = self.env.begin_rw_txn()?; - let mut block_signatures = txn - .get_value(self.block_metadata_db, &signature.block_hash)? - .unwrap_or_else(|| { - BlockSignatures::new(signature.block_hash, signature.era_id) - }); - block_signatures.insert_proof(signature.public_key, signature.signature); - let outcome = txn.put_value( - self.block_metadata_db, - &block_signatures.block_hash, - &block_signatures, - true, - )?; - txn.commit()?; - responder.respond(outcome).ignore() - } + } => responder + .respond(self.put_finality_signature(signature)?) + .ignore(), StorageRequest::GetBlockSignature { block_hash, public_key, @@ -1204,6 +1190,25 @@ impl Storage { }) } + fn put_finality_signature( + &mut self, + signature: Box, + ) -> Result { + let mut txn = self.env.begin_rw_txn()?; + let mut block_signatures = txn + .get_value(self.block_metadata_db, &signature.block_hash)? + .unwrap_or_else(|| BlockSignatures::new(signature.block_hash, signature.era_id)); + block_signatures.insert_proof(signature.public_key, signature.signature); + let outcome = txn.put_value( + self.block_metadata_db, + &block_signatures.block_hash, + &block_signatures, + true, + )?; + txn.commit()?; + Ok(outcome) + } + /// Handles a [`BlockCompletedAnnouncement`]. fn handle_mark_block_completed_request( &mut self, diff --git a/node/src/components/storage/tests.rs b/node/src/components/storage/tests.rs index 7be2b99ca4..0d5effef9d 100644 --- a/node/src/components/storage/tests.rs +++ b/node/src/components/storage/tests.rs @@ -1,9 +1,9 @@ //! Unit tests for the storage component. use std::{ - collections::{BTreeMap, HashMap}, + collections::{BTreeMap, BTreeSet, HashMap, HashSet}, fs::{self, File}, - iter, + iter::{self, FromIterator}, rc::Rc, sync::Arc, }; @@ -18,8 +18,8 @@ use casper_types::{ }; use super::{ - move_storage_files_to_network_subdir, should_move_storage_files_to_network_subdir, Config, - Storage, + initialize_block_metadata_db, move_storage_files_to_network_subdir, + should_move_storage_files_to_network_subdir, Config, Storage, }; use crate::{ components::fetcher::{FetchItem, FetchResponse}, @@ -1849,3 +1849,122 @@ fn unbonding_purse_serialization_roundtrip() { // Explicitly assert that the `new_validator` is not `None` assert!(deserialized.new_validator().is_some()) } + +// Clippy complains because there's a `OnceCell` in `FinalitySignature`, hence it should not be used +// as a key in `BTreeSet`. However, we don't change the content of the cell during the course of the +// test so there's no risk the hash or order of keys will change. +#[allow(clippy::mutable_key_type)] +fn assert_signatures(storage: &Storage, block_hash: BlockHash, expected: Vec) { + let mut txn = storage.env.begin_ro_txn().unwrap(); + let actual = storage + .get_block_signatures(&mut txn, &block_hash) + .expect("should be able to read signatures"); + let actual = actual.map_or(BTreeSet::new(), |signatures| { + signatures.finality_signatures().collect() + }); + let expected: BTreeSet<_> = expected.into_iter().collect(); + assert_eq!(actual, expected); +} + +#[test] +fn should_initialize_block_metadata_db() { + let mut harness = ComponentHarness::default(); + let mut storage = storage_fixture(&harness); + + let block_1 = Block::random(&mut harness.rng); + let fs_1_1 = + FinalitySignature::random_for_block(*block_1.hash(), block_1.header().era_id().into()); + let fs_1_2 = + FinalitySignature::random_for_block(*block_1.hash(), block_1.header().era_id().into()); + + let block_2 = Block::random(&mut harness.rng); + let fs_2_1 = + FinalitySignature::random_for_block(*block_2.hash(), block_2.header().era_id().into()); + let fs_2_2 = + FinalitySignature::random_for_block(*block_2.hash(), block_2.header().era_id().into()); + + let block_3 = Block::random(&mut harness.rng); + let fs_3_1 = + FinalitySignature::random_for_block(*block_3.hash(), block_3.header().era_id().into()); + let fs_3_2 = + FinalitySignature::random_for_block(*block_3.hash(), block_3.header().era_id().into()); + + let block_4 = Block::random(&mut harness.rng); + + let _ = storage.put_finality_signature(Box::new(fs_1_1.clone())); + let _ = storage.put_finality_signature(Box::new(fs_1_2.clone())); + let _ = storage.put_finality_signature(Box::new(fs_2_1.clone())); + let _ = storage.put_finality_signature(Box::new(fs_2_2.clone())); + let _ = storage.put_finality_signature(Box::new(fs_3_1.clone())); + let _ = storage.put_finality_signature(Box::new(fs_3_2.clone())); + + assert_signatures( + &storage, + *block_1.hash(), + vec![fs_1_1.clone(), fs_1_2.clone()], + ); + assert_signatures( + &storage, + *block_2.hash(), + vec![fs_2_1.clone(), fs_2_2.clone()], + ); + assert_signatures( + &storage, + *block_3.hash(), + vec![fs_3_1.clone(), fs_3_2.clone()], + ); + assert_signatures(&storage, *block_4.hash(), vec![]); + + // Purging empty set of blocks should not change state. + let to_be_purged = HashSet::new(); + let _ = initialize_block_metadata_db(&storage.env, &storage.block_metadata_db, &to_be_purged); + assert_signatures(&storage, *block_1.hash(), vec![fs_1_1, fs_1_2]); + assert_signatures( + &storage, + *block_2.hash(), + vec![fs_2_1.clone(), fs_2_2.clone()], + ); + assert_signatures( + &storage, + *block_3.hash(), + vec![fs_3_1.clone(), fs_3_2.clone()], + ); + + // Purging for block_1 should leave sigs for block_2 and block_3 intact. + let to_be_purged = HashSet::from_iter([block_1.hash().as_ref()]); + let _ = initialize_block_metadata_db(&storage.env, &storage.block_metadata_db, &to_be_purged); + assert_signatures(&storage, *block_1.hash(), vec![]); + assert_signatures( + &storage, + *block_2.hash(), + vec![fs_2_1.clone(), fs_2_2.clone()], + ); + assert_signatures( + &storage, + *block_3.hash(), + vec![fs_3_1.clone(), fs_3_2.clone()], + ); + assert_signatures(&storage, *block_4.hash(), vec![]); + + // Purging for block_4 (which has no signatures) should not modify state. + let to_be_purged = HashSet::from_iter([block_4.hash().as_ref()]); + let _ = initialize_block_metadata_db(&storage.env, &storage.block_metadata_db, &to_be_purged); + assert_signatures(&storage, *block_1.hash(), vec![]); + assert_signatures(&storage, *block_2.hash(), vec![fs_2_1, fs_2_2]); + assert_signatures(&storage, *block_3.hash(), vec![fs_3_1, fs_3_2]); + assert_signatures(&storage, *block_4.hash(), vec![]); + + // Purging for all blocks should leave no signatures. + let to_be_purged = HashSet::from_iter([ + block_1.hash().as_ref(), + block_2.hash().as_ref(), + block_3.hash().as_ref(), + block_4.hash().as_ref(), + ]); + + let _ = initialize_block_metadata_db(&storage.env, &storage.block_metadata_db, &to_be_purged); + assert_signatures(&storage, *block_1.hash(), vec![]); + assert_signatures(&storage, *block_2.hash(), vec![]); + assert_signatures(&storage, *block_3.hash(), vec![]); + assert_signatures(&storage, *block_4.hash(), vec![]); +} From b7d6ae767388a6ab3c60467d7adaaa6babba21a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Chabowski?= Date: Wed, 4 Oct 2023 12:16:33 +0200 Subject: [PATCH 3/3] Log hash of the blocks being purged as part of metadata db initialization --- node/src/components/storage.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/node/src/components/storage.rs b/node/src/components/storage.rs index 12eb37847f..bf8f1b34f4 100644 --- a/node/src/components/storage.rs +++ b/node/src/components/storage.rs @@ -3000,6 +3000,11 @@ fn initialize_block_metadata_db( let (raw_key, _) = row?; if deleted_block_hashes.contains(raw_key) { cursor.del(WriteFlags::empty())?; + let digest = Digest::try_from(raw_key); + debug!( + "purged metadata for block {}", + digest.map_or("".to_string(), |digest| digest.to_string()) + ); continue; } }