From 537e59558193a01fe140155fcf8bf417adc45c0b Mon Sep 17 00:00:00 2001 From: Dmitry Murzin Date: Tue, 16 Jul 2024 18:07:00 +0300 Subject: [PATCH] fix: Remove kura.lock (#4849) Signed-off-by: Dmitry Murzin --- core/benches/kura.rs | 4 +- core/src/kura.rs | 116 +++---------------------------- tools/kura_inspector/src/main.rs | 4 +- 3 files changed, 15 insertions(+), 109 deletions(-) diff --git a/core/benches/kura.rs b/core/benches/kura.rs index ad00f123533..b16213b684b 100644 --- a/core/benches/kura.rs +++ b/core/benches/kura.rs @@ -7,7 +7,7 @@ use criterion::{criterion_group, criterion_main, Criterion}; use iroha_config::{base::WithOrigin, parameters::actual::Kura as Config}; use iroha_core::{ block::*, - kura::{BlockStore, LockStatus}, + kura::BlockStore, prelude::*, query::store::LiveQueryStore, state::{State, World}, @@ -62,7 +62,7 @@ async fn measure_block_size_for_n_executors(n_executors: u32) { for _ in 1..n_executors { block.sign(&key_pair, &topology); } - let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); + let mut block_store = BlockStore::new(dir.path()); block_store.create_files_if_they_do_not_exist().unwrap(); block_store.append_block_to_chain(&block.into()).unwrap(); diff --git a/core/src/kura.rs b/core/src/kura.rs index 26589b07c16..bc094a4c5ab 100644 --- a/core/src/kura.rs +++ b/core/src/kura.rs @@ -4,7 +4,6 @@ //! blockchain. use std::{ fmt::Debug, - fs, io::{BufWriter, Read, Seek, SeekFrom, Write}, num::NonZeroUsize, path::{Path, PathBuf}, @@ -24,7 +23,6 @@ use crate::{block::CommittedBlock, handler::ThreadHandler}; const INDEX_FILE_NAME: &str = "blocks.index"; const DATA_FILE_NAME: &str = "blocks.data"; const HASHES_FILE_NAME: &str = "blocks.hashes"; -const LOCK_FILE_NAME: &str = "kura.lock"; const SIZE_OF_BLOCK_HASH: u64 = Hash::LENGTH as u64; @@ -52,7 +50,7 @@ impl Kura { /// path. pub fn new(config: &Config) -> Result<(Arc, BlockCount)> { let store_dir = config.store_dir.resolve_relative_path(); - let mut block_store = BlockStore::new(&store_dir, LockStatus::Unlocked); + let mut block_store = BlockStore::new(&store_dir); block_store.create_files_if_they_do_not_exist()?; let block_plain_text_path = config @@ -76,7 +74,7 @@ impl Kura { pub fn blank_kura_for_testing() -> Arc { Arc::new(Self { mode: InitMode::Strict, - block_store: Mutex::new(BlockStore::new(PathBuf::new(), LockStatus::Locked)), + block_store: Mutex::new(BlockStore::new(PathBuf::new())), block_data: Mutex::new(Vec::new()), block_plain_text_path: None, }) @@ -355,16 +353,6 @@ pub struct BlockStore { path_to_blockchain: PathBuf, } -impl Drop for BlockStore { - fn drop(&mut self) { - let path = self.path_to_blockchain.join(LOCK_FILE_NAME); - - if let Err(err) = fs::remove_file(path) { - error!(?err, "Failed to remove lock file"); - } - } -} - #[derive(Default, Debug, Clone, Copy)] /// Lightweight wrapper for block indices in the block index file pub struct BlockIndex { @@ -374,56 +362,9 @@ pub struct BlockIndex { pub length: u64, } -/// Locked Status -#[derive(Clone, Copy)] -pub enum LockStatus { - /// Is locked - Locked, - /// Is unlocked - Unlocked, -} - impl BlockStore { - /// Create a new read-only block store in `path`. - /// - /// # Panics - /// * if you pass in `LockStatus::Unlocked` and it is unable to lock the block store. - pub fn new(store_path: impl AsRef, lock_status: LockStatus) -> Self { - if matches!(lock_status, LockStatus::Unlocked) { - let lock_path = store_path.as_ref().join(LOCK_FILE_NAME); - if let Err(e) = fs::File::options() - .read(true) - .write(true) - .create_new(true) - .open(&lock_path) - { - match e.kind() { - std::io::ErrorKind::AlreadyExists => Err(Error::Locked(lock_path)), - std::io::ErrorKind::NotFound => { - match std::fs::create_dir_all(store_path.as_ref()) - .map_err(|e| Error::MkDir(e, store_path.as_ref().to_path_buf())) - { - Err(e) => Err(e), - Ok(()) => { - if let Err(e) = fs::File::options() - .read(true) - .write(true) - .create_new(true) - .open(&lock_path) - { - Err(Error::IO(e, lock_path)) - } else { - Ok(()) - } - } - } - } - _ => Err(Error::IO(e, lock_path)), - } - .expect("INTERNAL BUG: Kura must be able to lock the blockstore"); - } - } - + /// Create a new block store in `path`. + pub fn new(store_path: impl AsRef) -> Self { Self { path_to_blockchain: store_path.as_ref().to_path_buf(), } @@ -879,7 +820,7 @@ mod tests { #[test] fn read_and_write_to_blockchain_index() { let dir = tempfile::tempdir().unwrap(); - let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); + let mut block_store = BlockStore::new(dir.path()); block_store.create_files_if_they_do_not_exist().unwrap(); block_store.write_block_index(0, 5, 7).unwrap(); @@ -914,7 +855,7 @@ mod tests { #[test] fn read_and_write_to_blockchain_data_store() { let dir = tempfile::tempdir().unwrap(); - let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); + let mut block_store = BlockStore::new(dir.path()); block_store.create_files_if_they_do_not_exist().unwrap(); block_store @@ -930,7 +871,7 @@ mod tests { #[test] fn fresh_block_store_has_zero_blocks() { let dir = tempfile::tempdir().unwrap(); - let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); + let mut block_store = BlockStore::new(dir.path()); block_store.create_files_if_they_do_not_exist().unwrap(); assert_eq!(0, block_store.read_index_count().unwrap()); @@ -939,7 +880,7 @@ mod tests { #[test] fn append_block_to_chain_increases_block_count() { let dir = tempfile::tempdir().unwrap(); - let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); + let mut block_store = BlockStore::new(dir.path()); block_store.create_files_if_they_do_not_exist().unwrap(); let dummy_block = ValidBlock::new_dummy(KeyPair::random().private_key()).into(); @@ -955,7 +896,7 @@ mod tests { #[test] fn append_block_to_chain_increases_hashes_count() { let dir = tempfile::tempdir().unwrap(); - let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); + let mut block_store = BlockStore::new(dir.path()); block_store.create_files_if_they_do_not_exist().unwrap(); let dummy_block = ValidBlock::new_dummy(KeyPair::random().private_key()).into(); @@ -971,7 +912,7 @@ mod tests { #[test] fn append_block_to_chain_write_correct_hashes() { let dir = tempfile::tempdir().unwrap(); - let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); + let mut block_store = BlockStore::new(dir.path()); block_store.create_files_if_they_do_not_exist().unwrap(); let dummy_block = ValidBlock::new_dummy(KeyPair::random().private_key()).into(); @@ -991,7 +932,7 @@ mod tests { #[test] fn append_block_to_chain_places_blocks_correctly_in_data_file() { let dir = tempfile::tempdir().unwrap(); - let mut block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); + let mut block_store = BlockStore::new(dir.path()); block_store.create_files_if_they_do_not_exist().unwrap(); let dummy_block = ValidBlock::new_dummy(KeyPair::random().private_key()).into(); @@ -1009,41 +950,6 @@ mod tests { } } - #[test] - fn lock_and_unlock() { - let dir = tempfile::tempdir().unwrap(); - - { - let _store = BlockStore::new(dir.path(), LockStatus::Unlocked); - - assert!( - dir.path().join(LOCK_FILE_NAME).exists(), - "Lockfile should have been created" - ); - } - - assert!( - !dir.path().join(LOCK_FILE_NAME).exists(), - "Lockfile should have been deleted" - ); - } - - #[test] - #[should_panic(expected = "Kura must be able to lock the blockstore")] - fn concurrent_lock() { - let dir = tempfile::tempdir().unwrap(); - let _store = BlockStore::new(dir.path(), LockStatus::Unlocked); - let _store_2 = BlockStore::new(dir.path(), LockStatus::Unlocked); - } - - #[test] - fn unexpected_unlock() { - let dir = tempfile::tempdir().unwrap(); - let _block_store = BlockStore::new(dir.path(), LockStatus::Unlocked); - fs::remove_file(dir.path().join(LOCK_FILE_NAME)) - .expect("Lockfile should have been created"); - } - #[tokio::test] async fn strict_init_kura() { let temp_dir = TempDir::new().unwrap(); diff --git a/tools/kura_inspector/src/main.rs b/tools/kura_inspector/src/main.rs index c88e2ac7849..10eaec2efd6 100644 --- a/tools/kura_inspector/src/main.rs +++ b/tools/kura_inspector/src/main.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use clap::{Parser, Subcommand}; -use iroha_core::kura::{BlockIndex, BlockStore, LockStatus}; +use iroha_core::kura::{BlockIndex, BlockStore}; use iroha_data_model::block::SignedBlock; use iroha_version::scale::DecodeVersioned; @@ -59,7 +59,7 @@ fn print_blockchain(block_store_path: &Path, from_height: u64, block_count: u64) } } - let block_store = BlockStore::new(&block_store_path, LockStatus::Unlocked); + let block_store = BlockStore::new(&block_store_path); let index_count = block_store .read_index_count()