Skip to content

Commit

Permalink
fix: Remove kura.lock (#4849)
Browse files Browse the repository at this point in the history 
Signed-off-by: Dmitry Murzin <[email protected]>
  • Loading branch information
@dima74
dima74 authored Jul 16, 2024
1 parent bd2eba9 commit 537e595
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 109 deletions.
4 changes: 2 additions & 2 deletions core/benches/kura.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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();

Expand Down
116 changes: 11 additions & 105 deletions core/src/kura.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
//! blockchain.
use std::{
fmt::Debug,
fs,
io::{BufWriter, Read, Seek, SeekFrom, Write},
num::NonZeroUsize,
path::{Path, PathBuf},
Expand All @@ -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;

Expand Down Expand Up @@ -52,7 +50,7 @@ impl Kura {
/// path.
pub fn new(config: &Config) -> Result<(Arc<Self>, 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
Expand All @@ -76,7 +74,7 @@ impl Kura {
pub fn blank_kura_for_testing() -> Arc<Kura> {
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,
})
Expand Down Expand Up @@ -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 {
Expand All @@ -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<Path>, 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<Path>) -> Self {
Self {
path_to_blockchain: store_path.as_ref().to_path_buf(),
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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());
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions tools/kura_inspector/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 537e595

Please sign in to comment.