From 80895c5cb686c7c4e7e14441aa0e962e09fc7f9a Mon Sep 17 00:00:00 2001 From: Tpt Date: Tue, 18 Oct 2022 11:12:26 +0200 Subject: [PATCH 1/6] Fuzzer: Make sure to check that ref-counted not existing keys do not exist --- fuzz/fuzz_targets/refcounted_model.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/fuzz/fuzz_targets/refcounted_model.rs b/fuzz/fuzz_targets/refcounted_model.rs index 5bba06c0..5d59c9c2 100644 --- a/fuzz/fuzz_targets/refcounted_model.rs +++ b/fuzz/fuzz_targets/refcounted_model.rs @@ -174,8 +174,16 @@ impl DbSimulator for Simulator { } } - fn model_removed_content(_model: &Model) -> Vec> { - Vec::new() + fn model_removed_content(model: &Model) -> Vec> { + if let Some(last) = model.last() { + last.counts + .iter() + .enumerate() + .filter_map(|(k, count)| if count.is_some() { None } else { Some(vec![k as u8]) }) + .collect() + } else { + (u8::MIN..=u8::MAX).map(|k| vec![k]).collect() + } } } From 48695af01b27d1b758b0bb2ed06806da00aa9a10 Mon Sep 17 00:00:00 2001 From: Tpt Date: Tue, 18 Oct 2022 11:13:36 +0200 Subject: [PATCH 2/6] Fuzzer: Simplifies not ref-counted model Each layer contains the full state and not the changes --- fuzz/fuzz_targets/simple_model.rs | 71 +++++++++++-------------------- 1 file changed, 25 insertions(+), 46 deletions(-) diff --git a/fuzz/fuzz_targets/simple_model.rs b/fuzz/fuzz_targets/simple_model.rs index 2375f2ba..ed303fb1 100644 --- a/fuzz/fuzz_targets/simple_model.rs +++ b/fuzz/fuzz_targets/simple_model.rs @@ -14,7 +14,7 @@ const NUMBER_OF_POSSIBLE_KEYS: usize = 256; #[derive(Clone, Debug)] struct Layer { // For each key the value if it is inserted or None if it is removed - values: [Option>; NUMBER_OF_POSSIBLE_KEYS], + values: [Option; NUMBER_OF_POSSIBLE_KEYS], written: bool, } @@ -48,9 +48,9 @@ impl DbSimulator for Simulator { operations: impl IntoIterator)>, model: &mut Model, ) { - let mut values = [None; NUMBER_OF_POSSIBLE_KEYS]; + let mut values = model.last().map_or([None; NUMBER_OF_POSSIBLE_KEYS], |l| l.values); for (k, v) in operations { - values[usize::from(*k)] = Some(*v); + values[usize::from(*k)] = *v; } model.push(Layer { values, written: false }); } @@ -65,7 +65,6 @@ impl DbSimulator for Simulator { } fn attempt_to_reset_model_to_disk_state(model: &Model, state: &[(u8, u8)]) -> Option { - let mut model = model.clone(); let expected = { let mut values = [None; NUMBER_OF_POSSIBLE_KEYS]; for (k, v) in state { @@ -74,33 +73,19 @@ impl DbSimulator for Simulator { values }; - while !model.is_empty() { - if !model.last().unwrap().written { - model.pop(); + for layer in model.iter().rev() { + if !layer.written { continue } // Is it equal to current state? - let mut is_equal = true; - for (k, expected_value) in expected.iter().enumerate() { - for layer in model.iter().rev() { - if let Some(v) = layer.values[k] { - if v != *expected_value { - is_equal = false; - } - break - } - } - } - if is_equal { - // We found it! - return Some(model) + if layer.values == expected { + // We found a correct last layer + return Some(vec![layer.clone()]) } - log::debug!("Reverting layer number {}", model.len() - 1); - model.pop(); } if state.is_empty() { - Some(Vec::new()) // empty state + Some(Vec::new()) } else { None } @@ -116,18 +101,15 @@ impl DbSimulator for Simulator { } fn model_required_content(model: &Model) -> Vec<(Vec, Vec)> { - let mut content = Vec::new(); - for k in u8::MIN..=u8::MAX { - for layer in model.iter().rev() { - if let Some(v) = layer.values[usize::from(k)] { - if let Some(v) = v { - content.push((vec![k], vec![v])); - } - break - } - } + if let Some(last) = model.last() { + last.values + .iter() + .enumerate() + .filter_map(|(i, v)| v.map(|v| (vec![i as u8], vec![v]))) + .collect() + } else { + Vec::new() } - content } fn model_optional_content(model: &Model) -> Vec<(Vec, Vec)> { @@ -135,18 +117,15 @@ impl DbSimulator for Simulator { } fn model_removed_content(model: &Model) -> Vec> { - let mut keys = Vec::new(); - for k in u8::MIN..=u8::MAX { - for layer in model.iter().rev() { - if let Some(v) = layer.values[usize::from(k)] { - if v.is_none() { - keys.push(vec![k]); - } - break - } - } + if let Some(last) = model.last() { + last.values + .iter() + .enumerate() + .filter_map(|(i, v)| if v.is_none() { Some(vec![i as u8]) } else { None }) + .collect() + } else { + (u8::MIN..=u8::MAX).map(|k| vec![k]).collect() } - keys } } From 99e6806db990cf86a05efe14c7e0e13c3f03fda3 Mon Sep 17 00:00:00 2001 From: Tpt Date: Tue, 18 Oct 2022 11:18:13 +0200 Subject: [PATCH 3/6] Fuzzer: Avoids options duplicated code and set salt Makes reproduction easier (consistent hashing) --- fuzz/fuzz_targets/refcounted_model.rs | 26 ++++++++------------------ fuzz/fuzz_targets/simple_model.rs | 21 +++++---------------- fuzz/src/lib.rs | 16 +++++++++++++--- 3 files changed, 26 insertions(+), 37 deletions(-) diff --git a/fuzz/fuzz_targets/refcounted_model.rs b/fuzz/fuzz_targets/refcounted_model.rs index 5d59c9c2..6c718c5f 100644 --- a/fuzz/fuzz_targets/refcounted_model.rs +++ b/fuzz/fuzz_targets/refcounted_model.rs @@ -8,7 +8,7 @@ use arbitrary::Arbitrary; use libfuzzer_sys::fuzz_target; use parity_db_fuzz::*; -use std::{cmp::min, collections::HashMap, path::Path}; +use std::cmp::min; const NUMBER_OF_POSSIBLE_KEYS: usize = 256; @@ -34,23 +34,13 @@ impl DbSimulator for Simulator { type Operation = Operation; type Model = Model; - fn build_options(config: &Config, path: &Path) -> parity_db::Options { - parity_db::Options { - path: path.to_owned(), - columns: vec![parity_db::ColumnOptions { - ref_counted: true, - preimage: true, - compression: config.compression.into(), - btree_index: config.btree_index, - ..parity_db::ColumnOptions::default() - }], - sync_wal: true, - sync_data: true, - stats: false, - salt: None, - compression_threshold: HashMap::new(), - always_flush: true, - with_background_thread: false, + fn build_column_options(config: &Config) -> parity_db::ColumnOptions { + parity_db::ColumnOptions { + ref_counted: true, + preimage: true, + compression: config.compression.into(), + btree_index: config.btree_index, + ..parity_db::ColumnOptions::default() } } diff --git a/fuzz/fuzz_targets/simple_model.rs b/fuzz/fuzz_targets/simple_model.rs index ed303fb1..fa7dc52a 100644 --- a/fuzz/fuzz_targets/simple_model.rs +++ b/fuzz/fuzz_targets/simple_model.rs @@ -7,7 +7,6 @@ #![no_main] use libfuzzer_sys::fuzz_target; use parity_db_fuzz::*; -use std::{collections::HashMap, path::Path}; const NUMBER_OF_POSSIBLE_KEYS: usize = 256; @@ -26,21 +25,11 @@ impl DbSimulator for Simulator { type Operation = (u8, Option); type Model = Model; - fn build_options(config: &Config, path: &Path) -> parity_db::Options { - parity_db::Options { - path: path.to_owned(), - columns: vec![parity_db::ColumnOptions { - compression: config.compression.into(), - btree_index: config.btree_index, - ..parity_db::ColumnOptions::default() - }], - sync_wal: true, - sync_data: true, - stats: false, - salt: None, - compression_threshold: HashMap::new(), - always_flush: true, - with_background_thread: false, + fn build_column_options(config: &Config) -> parity_db::ColumnOptions { + parity_db::ColumnOptions { + compression: config.compression.into(), + btree_index: config.btree_index, + ..parity_db::ColumnOptions::default() } } diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 4c6a1ecf..2ef3a655 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -2,7 +2,7 @@ // This file is dual-licensed as Apache-2.0 or MIT. use arbitrary::Arbitrary; -use std::{cmp::Ordering, fmt::Debug, path::Path}; +use std::{cmp::Ordering, collections::HashMap, fmt::Debug}; use tempfile::tempdir; #[derive(Arbitrary, Debug, Clone, Copy)] @@ -43,7 +43,7 @@ pub trait DbSimulator { type Operation: Debug; type Model: Default + Debug; - fn build_options(config: &Config, path: &Path) -> parity_db::Options; + fn build_column_options(config: &Config) -> parity_db::ColumnOptions; fn apply_operations_on_model<'a>( operations: impl IntoIterator, @@ -68,7 +68,17 @@ pub trait DbSimulator { fn simulate(config: Config, actions: Vec>) { let dir = tempdir().unwrap(); - let options = Self::build_options(&config, dir.path()); + let options = parity_db::Options { + path: dir.path().to_owned(), + columns: vec![Self::build_column_options(&config)], + sync_wal: true, + sync_data: true, + stats: false, + salt: Some([0; 32]), + compression_threshold: HashMap::new(), + always_flush: true, + with_background_thread: false, + }; // We don't check for now failures inside of initialization. parity_db::set_number_of_allowed_io_operations(usize::MAX); From a917267ef6dc5189c7bfb9e707fdeba218541094 Mon Sep 17 00:00:00 2001 From: Tpt Date: Tue, 18 Oct 2022 13:01:52 +0200 Subject: [PATCH 4/6] Fuzzer: Removes duplicated code related to layers --- fuzz/fuzz_targets/refcounted_model.rs | 119 ++++++++++---------------- fuzz/fuzz_targets/simple_model.rs | 71 +++++---------- fuzz/src/lib.rs | 76 +++++++++------- 3 files changed, 110 insertions(+), 156 deletions(-) diff --git a/fuzz/fuzz_targets/refcounted_model.rs b/fuzz/fuzz_targets/refcounted_model.rs index 6c718c5f..f6626ca1 100644 --- a/fuzz/fuzz_targets/refcounted_model.rs +++ b/fuzz/fuzz_targets/refcounted_model.rs @@ -10,8 +10,6 @@ use libfuzzer_sys::fuzz_target; use parity_db_fuzz::*; use std::cmp::min; -const NUMBER_OF_POSSIBLE_KEYS: usize = 256; - #[derive(Arbitrary, Debug, Clone, Copy)] enum Operation { Set(u8), @@ -19,20 +17,11 @@ enum Operation { Reference(u8), } -#[derive(Clone, Debug)] -struct Layer { - // The number of references per key (or None if the key never existed in the database) - counts: [Option; NUMBER_OF_POSSIBLE_KEYS], - written: bool, -} - -type Model = Vec; - struct Simulator; impl DbSimulator for Simulator { + type ValueType = usize; type Operation = Operation; - type Model = Model; fn build_column_options(config: &Config) -> parity_db::ColumnOptions { parity_db::ColumnOptions { @@ -44,38 +33,26 @@ impl DbSimulator for Simulator { } } - fn apply_operations_on_model<'a>( - operations: impl IntoIterator, - model: &mut Model, + fn apply_operations_on_values<'a>( + operations: impl IntoIterator, + values: &mut [Option; NUMBER_OF_POSSIBLE_KEYS], ) { - let mut counts = model.last().map_or([None; NUMBER_OF_POSSIBLE_KEYS], |l| l.counts); for operation in operations { match *operation { - Operation::Set(k) => *counts[usize::from(k)].get_or_insert(0) += 1, + Operation::Set(k) => *values[usize::from(k)].get_or_insert(0) += 1, Operation::Dereference(k) => - if counts[usize::from(k)].unwrap_or(0) > 0 { - *counts[usize::from(k)].get_or_insert(0) -= 1; + if values[usize::from(k)].unwrap_or(0) > 0 { + *values[usize::from(k)].get_or_insert(0) -= 1; }, Operation::Reference(k) => - if counts[usize::from(k)].unwrap_or(0) > 0 { - *counts[usize::from(k)].get_or_insert(0) += 1; + if values[usize::from(k)].unwrap_or(0) > 0 { + *values[usize::from(k)].get_or_insert(0) += 1; }, } } - - model.push(Layer { counts, written: false }); - } - - fn write_first_layer_to_disk(model: &mut Model) { - for layer in model { - if !layer.written { - layer.written = true; - break - } - } } - fn attempt_to_reset_model_to_disk_state(model: &Model, state: &[(u8, u8)]) -> Option { + fn attempt_to_reset_model_to_disk_state(layers: &[Layer], state: &[(u8, u8)]) -> Option>> { let expected = { let mut is_present = [false; NUMBER_OF_POSSIBLE_KEYS]; for (k, _) in state { @@ -85,7 +62,7 @@ impl DbSimulator for Simulator { }; let mut candidates = Vec::new(); - for layer in model.iter().rev() { + for layer in layers.iter().rev() { if !layer.written { continue } @@ -93,9 +70,9 @@ impl DbSimulator for Simulator { // Is it equal to current state? let is_equal = expected.iter().enumerate().all(|(k, is_present)| { if *is_present { - layer.counts[k].is_some() + layer.values[k].is_some() } else { - layer.counts[k].unwrap_or(0) == 0 + layer.values[k].unwrap_or(0) == 0 } }); if is_equal { @@ -111,13 +88,13 @@ impl DbSimulator for Simulator { let mut new_state_safe_counts = [None; NUMBER_OF_POSSIBLE_KEYS]; for layer in candidates { for i in u8::MIN..=u8::MAX { - if let Some(c) = layer.counts[usize::from(i)] { + if let Some(c) = layer.values[usize::from(i)] { new_state_safe_counts[usize::from(i)] = Some(min(c, new_state_safe_counts[usize::from(i)].unwrap_or(usize::MAX))); } } } - Some(vec![Layer { counts: new_state_safe_counts, written: true }]) + Some(vec![Layer { values: new_state_safe_counts, written: true }]) } fn map_operation(operation: &Operation) -> parity_db::Operation, Vec> { @@ -128,52 +105,44 @@ impl DbSimulator for Simulator { } } - fn model_required_content(model: &Model) -> Vec<(Vec, Vec)> { - if let Some(last) = model.last() { - last.counts - .iter() - .enumerate() - .filter_map(|(k, count)| { - if count.unwrap_or(0) > 0 { - Some((vec![k as u8], vec![k as u8])) - } else { - None - } - }) - .collect() - } else { - Vec::new() - } + fn layer_required_content( + values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + ) -> Vec<(Vec, Vec)> { + values + .iter() + .enumerate() + .filter_map(|(k, count)| { + if count.unwrap_or(0) > 0 { + Some((vec![k as u8], vec![k as u8])) + } else { + None + } + }) + .collect() } - fn model_optional_content(model: &Model) -> Vec<(Vec, Vec)> { - if let Some(last) = model.last() { - last.counts - .iter() - .enumerate() - .filter_map(|(k, count)| { + fn layer_optional_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)> { + values + .iter() + .enumerate() + .filter_map( + |(k, count)| { if count.is_some() { Some((vec![k as u8], vec![k as u8])) } else { None } - }) - .collect() - } else { - Vec::new() - } + }, + ) + .collect() } - fn model_removed_content(model: &Model) -> Vec> { - if let Some(last) = model.last() { - last.counts - .iter() - .enumerate() - .filter_map(|(k, count)| if count.is_some() { None } else { Some(vec![k as u8]) }) - .collect() - } else { - (u8::MIN..=u8::MAX).map(|k| vec![k]).collect() - } + fn layer_removed_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec> { + values + .iter() + .enumerate() + .filter_map(|(k, count)| if count.is_some() { None } else { Some(vec![k as u8]) }) + .collect() } } diff --git a/fuzz/fuzz_targets/simple_model.rs b/fuzz/fuzz_targets/simple_model.rs index fa7dc52a..69c8e68a 100644 --- a/fuzz/fuzz_targets/simple_model.rs +++ b/fuzz/fuzz_targets/simple_model.rs @@ -7,23 +7,11 @@ #![no_main] use libfuzzer_sys::fuzz_target; use parity_db_fuzz::*; - -const NUMBER_OF_POSSIBLE_KEYS: usize = 256; - -#[derive(Clone, Debug)] -struct Layer { - // For each key the value if it is inserted or None if it is removed - values: [Option; NUMBER_OF_POSSIBLE_KEYS], - written: bool, -} - -type Model = Vec; - struct Simulator; impl DbSimulator for Simulator { + type ValueType = u8; type Operation = (u8, Option); - type Model = Model; fn build_column_options(config: &Config) -> parity_db::ColumnOptions { parity_db::ColumnOptions { @@ -33,27 +21,16 @@ impl DbSimulator for Simulator { } } - fn apply_operations_on_model<'a>( - operations: impl IntoIterator)>, - model: &mut Model, + fn apply_operations_on_values<'a>( + operations: impl IntoIterator, + values: &mut [Option; NUMBER_OF_POSSIBLE_KEYS], ) { - let mut values = model.last().map_or([None; NUMBER_OF_POSSIBLE_KEYS], |l| l.values); for (k, v) in operations { values[usize::from(*k)] = *v; } - model.push(Layer { values, written: false }); - } - - fn write_first_layer_to_disk(model: &mut Model) { - for layer in model { - if !layer.written { - layer.written = true; - break - } - } } - fn attempt_to_reset_model_to_disk_state(model: &Model, state: &[(u8, u8)]) -> Option { + fn attempt_to_reset_model_to_disk_state(layers: &[Layer], state: &[(u8, u8)]) -> Option>> { let expected = { let mut values = [None; NUMBER_OF_POSSIBLE_KEYS]; for (k, v) in state { @@ -62,7 +39,7 @@ impl DbSimulator for Simulator { values }; - for layer in model.iter().rev() { + for layer in layers.iter().rev() { if !layer.written { continue } @@ -89,32 +66,24 @@ impl DbSimulator for Simulator { } } - fn model_required_content(model: &Model) -> Vec<(Vec, Vec)> { - if let Some(last) = model.last() { - last.values - .iter() - .enumerate() - .filter_map(|(i, v)| v.map(|v| (vec![i as u8], vec![v]))) - .collect() - } else { - Vec::new() - } + fn layer_required_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)> { + values + .iter() + .enumerate() + .filter_map(|(i, v)| v.map(|v| (vec![i as u8], vec![v]))) + .collect() } - fn model_optional_content(model: &Model) -> Vec<(Vec, Vec)> { - Self::model_required_content(model) + fn layer_optional_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)> { + Self::layer_required_content(values) } - fn model_removed_content(model: &Model) -> Vec> { - if let Some(last) = model.last() { - last.values - .iter() - .enumerate() - .filter_map(|(i, v)| if v.is_none() { Some(vec![i as u8]) } else { None }) - .collect() - } else { - (u8::MIN..=u8::MAX).map(|k| vec![k]).collect() - } + fn layer_removed_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec> { + values + .iter() + .enumerate() + .filter_map(|(i, v)| if v.is_none() { Some(vec![i as u8]) } else { None }) + .collect() } } diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 2ef3a655..1dcad59c 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -5,6 +5,8 @@ use arbitrary::Arbitrary; use std::{cmp::Ordering, collections::HashMap, fmt::Debug}; use tempfile::tempdir; +pub const NUMBER_OF_POSSIBLE_KEYS: usize = 256; + #[derive(Arbitrary, Debug, Clone, Copy)] pub enum CompressionType { NoCompression, @@ -39,32 +41,37 @@ pub enum Action { Restart, } +#[derive(Clone, Debug)] +pub struct Layer { + // The stored value per possible key (depends if we have ref counting or not) + pub values: [Option; NUMBER_OF_POSSIBLE_KEYS], + pub written: bool, +} + pub trait DbSimulator { + type ValueType: Debug + Copy; type Operation: Debug; - type Model: Default + Debug; fn build_column_options(config: &Config) -> parity_db::ColumnOptions; - fn apply_operations_on_model<'a>( + fn apply_operations_on_values<'a>( operations: impl IntoIterator, - model: &mut Self::Model, + values: &mut [Option; NUMBER_OF_POSSIBLE_KEYS], ) where Self::Operation: 'a; - fn write_first_layer_to_disk(model: &mut Self::Model); - fn attempt_to_reset_model_to_disk_state( - model: &Self::Model, + layers: &[Layer], state: &[(u8, u8)], - ) -> Option; + ) -> Option>>; fn map_operation(operation: &Self::Operation) -> parity_db::Operation, Vec>; - fn model_required_content(model: &Self::Model) -> Vec<(Vec, Vec)>; + fn layer_required_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)>; - fn model_optional_content(model: &Self::Model) -> Vec<(Vec, Vec)>; + fn layer_optional_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)>; - fn model_removed_content(model: &Self::Model) -> Vec>; + fn layer_removed_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec>; fn simulate(config: Config, actions: Vec>) { let dir = tempdir().unwrap(); @@ -83,7 +90,7 @@ pub trait DbSimulator { // We don't check for now failures inside of initialization. parity_db::set_number_of_allowed_io_operations(usize::MAX); let mut db = parity_db::Db::open_or_create(&options).unwrap(); - let mut model = Self::Model::default(); + let mut layers = Vec::new(); parity_db::set_number_of_allowed_io_operations( config.number_of_allowed_io_operations.into(), ); @@ -92,36 +99,44 @@ pub trait DbSimulator { log::debug!("Applying on the database: {:?}", action); match action { Action::Transaction(operations) => { - Self::apply_operations_on_model(operations, &mut model); + let mut values = + layers.last().map_or([None; NUMBER_OF_POSSIBLE_KEYS], |l: &Layer| l.values); + Self::apply_operations_on_values(operations, &mut values); + layers.push(Layer { values, written: false }); db.commit_changes(operations.iter().map(|o| (0, Self::map_operation(o)))) .unwrap(); }, Action::ProcessReindex => - db = Self::try_or_restart(|db| db.process_reindex(), db, &mut model, &options), + db = Self::try_or_restart(|db| db.process_reindex(), db, &mut layers, &options), Action::ProcessCommits => { - Self::write_first_layer_to_disk(&mut model); - db = Self::try_or_restart(|db| db.process_commits(), db, &mut model, &options) + for layer in &mut layers { + if !layer.written { + layer.written = true; + break + } + } + db = Self::try_or_restart(|db| db.process_commits(), db, &mut layers, &options) }, Action::FlushAndEnactLogs => { // We repeat flush and then call enact_log to avoid deadlocks due to // Log::flush_one side effects for _ in 0..2 { - db = Self::try_or_restart(|db| db.flush_logs(), db, &mut model, &options) + db = Self::try_or_restart(|db| db.flush_logs(), db, &mut layers, &options) } - db = Self::try_or_restart(|db| db.enact_logs(), db, &mut model, &options) + db = Self::try_or_restart(|db| db.enact_logs(), db, &mut layers, &options) }, Action::CleanLogs => - db = Self::try_or_restart(|db| db.clean_logs(), db, &mut model, &options), + db = Self::try_or_restart(|db| db.clean_logs(), db, &mut layers, &options), Action::Restart => { db = { drop(db); retry_operation(|| parity_db::Db::open(&options)) }; - Self::reset_model_from_database(&db, &mut model); + Self::reset_model_from_database(&db, &mut layers); }, } retry_operation(|| { - Self::check_db_and_model_are_equals(&db, &model, config.btree_index) + Self::check_db_and_model_are_equals(&db, &layers, config.btree_index) }) .unwrap(); } @@ -130,7 +145,7 @@ pub trait DbSimulator { fn try_or_restart( op: impl FnOnce(&parity_db::Db) -> parity_db::Result<()>, mut db: parity_db::Db, - model: &mut Self::Model, + layers: &mut Vec>, options: &parity_db::Options, ) -> parity_db::Db { match op(&db) { @@ -140,15 +155,15 @@ pub trait DbSimulator { drop(db); parity_db::set_number_of_allowed_io_operations(usize::MAX); db = parity_db::Db::open(options).unwrap(); - Self::reset_model_from_database(&db, model); + Self::reset_model_from_database(&db, layers); db }, Err(e) => panic!("database error: {}", e), } } - fn reset_model_from_database(db: &parity_db::Db, model: &mut Self::Model) { - *model = retry_operation(|| { + fn reset_model_from_database(db: &parity_db::Db, layers: &mut Vec>) { + *layers = retry_operation(|| { let mut disk_state = Vec::new(); for i in u8::MIN..=u8::MAX { if let Some(v) = db.get(0, &[i])? { @@ -156,8 +171,8 @@ pub trait DbSimulator { } } - if let Some(model) = Self::attempt_to_reset_model_to_disk_state(model, &disk_state) { - Ok(model) + if let Some(layers) = Self::attempt_to_reset_model_to_disk_state(layers, &disk_state) { + Ok(layers) } else { Err(parity_db::Error::Corruption(format!("Not able to recover the database to one of the valid state. The current database state is: {:?}", disk_state))) } @@ -166,22 +181,23 @@ pub trait DbSimulator { fn check_db_and_model_are_equals( db: &parity_db::Db, - model: &Self::Model, + layers: &[Layer], is_db_b_tree: bool, ) -> parity_db::Result> { - for (k, v) in Self::model_required_content(model) { + let values = layers.last().map_or([None; NUMBER_OF_POSSIBLE_KEYS], |l| l.values); + for (k, v) in Self::layer_required_content(&values) { if db.get(0, &k)?.as_ref() != Some(&v) { return Ok(Err(format!("The value {:?} for key {:?} is not in the database", v, k))) } } - for k in Self::model_removed_content(model) { + for k in Self::layer_removed_content(&values) { if db.get(0, &k)?.is_some() { return Ok(Err(format!("The key {:?} should not be in the database", k))) } } if is_db_b_tree { - let mut model_content = Self::model_optional_content(model); + let mut model_content = Self::layer_optional_content(&values); // We check the BTree forward iterator let mut db_iter = db.iter(0)?; From 8b28514cd6b4a72fff5327e68af253d932c9bf04 Mon Sep 17 00:00:00 2001 From: Tpt Date: Tue, 18 Oct 2022 13:18:19 +0200 Subject: [PATCH 5/6] Fuzzer: Removes duplication related to recovery --- fuzz/fuzz_targets/refcounted_model.rs | 54 +++++++++++---------------- fuzz/fuzz_targets/simple_model.rs | 45 ++++++++++------------ fuzz/src/lib.rs | 53 ++++++++++++++++++++++---- 3 files changed, 86 insertions(+), 66 deletions(-) diff --git a/fuzz/fuzz_targets/refcounted_model.rs b/fuzz/fuzz_targets/refcounted_model.rs index f6626ca1..10f8464e 100644 --- a/fuzz/fuzz_targets/refcounted_model.rs +++ b/fuzz/fuzz_targets/refcounted_model.rs @@ -52,41 +52,27 @@ impl DbSimulator for Simulator { } } - fn attempt_to_reset_model_to_disk_state(layers: &[Layer], state: &[(u8, u8)]) -> Option>> { - let expected = { - let mut is_present = [false; NUMBER_OF_POSSIBLE_KEYS]; - for (k, _) in state { - is_present[usize::from(*k)] = true; - } - is_present - }; - - let mut candidates = Vec::new(); - for layer in layers.iter().rev() { - if !layer.written { - continue - } - - // Is it equal to current state? - let is_equal = expected.iter().enumerate().all(|(k, is_present)| { - if *is_present { - layer.values[k].is_some() - } else { - layer.values[k].unwrap_or(0) == 0 - } - }); - if is_equal { - // We found a correct last layer - candidates.push(layer); - } - } - if candidates.is_empty() { - return if state.is_empty() { Some(Vec::new()) } else { None } + fn is_layer_state_compatible_with_disk_state( + layer_values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + state: &[(u8, u8)], + ) -> bool { + if !state.iter().all(|(k, v)| k == v) { + return false // keys and values should be equal } + layer_values.iter().enumerate().all(|(i, c)| { + let key = i as u8; + match c { + None => state.iter().all(|(k, _)| *k != key), + Some(0) => true, + Some(_) => state.iter().any(|(k, _)| *k == key), + } + }) + } + fn build_best_layer_for_recovery(layers: &[&Layer]) -> Layer { // if we are multiple candidates, we are unsure. We pick the lower count per candidate let mut new_state_safe_counts = [None; NUMBER_OF_POSSIBLE_KEYS]; - for layer in candidates { + for layer in layers { for i in u8::MIN..=u8::MAX { if let Some(c) = layer.values[usize::from(i)] { new_state_safe_counts[usize::from(i)] = @@ -94,7 +80,7 @@ impl DbSimulator for Simulator { } } } - Some(vec![Layer { values: new_state_safe_counts, written: true }]) + Layer { values: new_state_safe_counts, written: true } } fn map_operation(operation: &Operation) -> parity_db::Operation, Vec> { @@ -121,7 +107,9 @@ impl DbSimulator for Simulator { .collect() } - fn layer_optional_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)> { + fn layer_optional_content( + values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + ) -> Vec<(Vec, Vec)> { values .iter() .enumerate() diff --git a/fuzz/fuzz_targets/simple_model.rs b/fuzz/fuzz_targets/simple_model.rs index 69c8e68a..26e139e7 100644 --- a/fuzz/fuzz_targets/simple_model.rs +++ b/fuzz/fuzz_targets/simple_model.rs @@ -30,31 +30,22 @@ impl DbSimulator for Simulator { } } - fn attempt_to_reset_model_to_disk_state(layers: &[Layer], state: &[(u8, u8)]) -> Option>> { - let expected = { - let mut values = [None; NUMBER_OF_POSSIBLE_KEYS]; - for (k, v) in state { - values[usize::from(*k)] = Some(*v); - } - values - }; - - for layer in layers.iter().rev() { - if !layer.written { - continue + fn is_layer_state_compatible_with_disk_state( + layer_values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + state: &[(u8, u8)], + ) -> bool { + layer_values.iter().enumerate().all(|(i, value)| { + let key = i as u8; + if let Some(value) = value { + state.iter().any(|(k, v)| *k == key && v == value) + } else { + state.iter().all(|(k, _)| *k != key) } + }) + } - // Is it equal to current state? - if layer.values == expected { - // We found a correct last layer - return Some(vec![layer.clone()]) - } - } - if state.is_empty() { - Some(Vec::new()) - } else { - None - } + fn build_best_layer_for_recovery(layers: &[&Layer]) -> Layer { + layers[0].clone() } fn map_operation(operation: &(u8, Option)) -> parity_db::Operation, Vec> { @@ -66,7 +57,9 @@ impl DbSimulator for Simulator { } } - fn layer_required_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)> { + fn layer_required_content( + values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + ) -> Vec<(Vec, Vec)> { values .iter() .enumerate() @@ -74,7 +67,9 @@ impl DbSimulator for Simulator { .collect() } - fn layer_optional_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)> { + fn layer_optional_content( + values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + ) -> Vec<(Vec, Vec)> { Self::layer_required_content(values) } diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 1dcad59c..511b80e4 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -60,18 +60,26 @@ pub trait DbSimulator { ) where Self::Operation: 'a; - fn attempt_to_reset_model_to_disk_state( - layers: &[Layer], + fn is_layer_state_compatible_with_disk_state( + layer_values: &[Option; NUMBER_OF_POSSIBLE_KEYS], state: &[(u8, u8)], - ) -> Option>>; + ) -> bool; + + fn build_best_layer_for_recovery(layers: &[&Layer]) -> Layer; fn map_operation(operation: &Self::Operation) -> parity_db::Operation, Vec>; - fn layer_required_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)>; + fn layer_required_content( + values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + ) -> Vec<(Vec, Vec)>; - fn layer_optional_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec<(Vec, Vec)>; + fn layer_optional_content( + values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + ) -> Vec<(Vec, Vec)>; - fn layer_removed_content(values: &[Option; NUMBER_OF_POSSIBLE_KEYS]) -> Vec>; + fn layer_removed_content( + values: &[Option; NUMBER_OF_POSSIBLE_KEYS], + ) -> Vec>; fn simulate(config: Config, actions: Vec>) { let dir = tempdir().unwrap(); @@ -99,8 +107,11 @@ pub trait DbSimulator { log::debug!("Applying on the database: {:?}", action); match action { Action::Transaction(operations) => { - let mut values = - layers.last().map_or([None; NUMBER_OF_POSSIBLE_KEYS], |l: &Layer| l.values); + let mut values = layers + .last() + .map_or([None; NUMBER_OF_POSSIBLE_KEYS], |l: &Layer| { + l.values + }); Self::apply_operations_on_values(operations, &mut values); layers.push(Layer { values, written: false }); db.commit_changes(operations.iter().map(|o| (0, Self::map_operation(o)))) @@ -179,6 +190,32 @@ pub trait DbSimulator { }) } + fn attempt_to_reset_model_to_disk_state( + layers: &[Layer], + state: &[(u8, u8)], + ) -> Option>> { + let mut candidates = Vec::new(); + for layer in layers.iter().rev() { + if !layer.written { + continue + } + + if Self::is_layer_state_compatible_with_disk_state(&layer.values, state) { + // We found a correct last layer + candidates.push(layer); + } + } + if candidates.is_empty() { + if state.is_empty() { + Some(Vec::new()) + } else { + None + } + } else { + Some(vec![Self::build_best_layer_for_recovery(&candidates)]) + } + } + fn check_db_and_model_are_equals( db: &parity_db::Db, layers: &[Layer], From 927c724bda67a0064ad1b2aabe291fc2bf711de6 Mon Sep 17 00:00:00 2001 From: Tpt Date: Tue, 18 Oct 2022 13:49:38 +0200 Subject: [PATCH 6/6] Fuzzer: If the disk is disconnected during recovery, the next restart might lead to a previous state --- fuzz/src/lib.rs | 67 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/fuzz/src/lib.rs b/fuzz/src/lib.rs index 511b80e4..40fdc602 100644 --- a/fuzz/src/lib.rs +++ b/fuzz/src/lib.rs @@ -99,6 +99,9 @@ pub trait DbSimulator { parity_db::set_number_of_allowed_io_operations(usize::MAX); let mut db = parity_db::Db::open_or_create(&options).unwrap(); let mut layers = Vec::new(); + // In case of bad writes, when restarting the DB we might end up to with a state of the + // previous opening but with a state of a older one. + let mut old_layers = Vec::new(); parity_db::set_number_of_allowed_io_operations( config.number_of_allowed_io_operations.into(), ); @@ -118,7 +121,13 @@ pub trait DbSimulator { .unwrap(); }, Action::ProcessReindex => - db = Self::try_or_restart(|db| db.process_reindex(), db, &mut layers, &options), + db = Self::try_or_restart( + |db| db.process_reindex(), + db, + &mut layers, + &old_layers, + &options, + ), Action::ProcessCommits => { for layer in &mut layers { if !layer.written { @@ -126,24 +135,49 @@ pub trait DbSimulator { break } } - db = Self::try_or_restart(|db| db.process_commits(), db, &mut layers, &options) + db = Self::try_or_restart( + |db| db.process_commits(), + db, + &mut layers, + &old_layers, + &options, + ) }, Action::FlushAndEnactLogs => { // We repeat flush and then call enact_log to avoid deadlocks due to // Log::flush_one side effects for _ in 0..2 { - db = Self::try_or_restart(|db| db.flush_logs(), db, &mut layers, &options) + db = Self::try_or_restart( + |db| db.flush_logs(), + db, + &mut layers, + &old_layers, + &options, + ) } - db = Self::try_or_restart(|db| db.enact_logs(), db, &mut layers, &options) + db = Self::try_or_restart( + |db| db.enact_logs(), + db, + &mut layers, + &old_layers, + &options, + ) }, Action::CleanLogs => - db = Self::try_or_restart(|db| db.clean_logs(), db, &mut layers, &options), + db = Self::try_or_restart( + |db| db.clean_logs(), + db, + &mut layers, + &old_layers, + &options, + ), Action::Restart => { + old_layers.push(layers.clone()); db = { drop(db); retry_operation(|| parity_db::Db::open(&options)) }; - Self::reset_model_from_database(&db, &mut layers); + Self::reset_model_from_database(&db, &mut layers, &old_layers); }, } retry_operation(|| { @@ -157,6 +191,7 @@ pub trait DbSimulator { op: impl FnOnce(&parity_db::Db) -> parity_db::Result<()>, mut db: parity_db::Db, layers: &mut Vec>, + old_layers: &[Vec>], options: &parity_db::Options, ) -> parity_db::Db { match op(&db) { @@ -166,14 +201,18 @@ pub trait DbSimulator { drop(db); parity_db::set_number_of_allowed_io_operations(usize::MAX); db = parity_db::Db::open(options).unwrap(); - Self::reset_model_from_database(&db, layers); + Self::reset_model_from_database(&db, layers, old_layers); db }, Err(e) => panic!("database error: {}", e), } } - fn reset_model_from_database(db: &parity_db::Db, layers: &mut Vec>) { + fn reset_model_from_database( + db: &parity_db::Db, + layers: &mut Vec>, + old_layers: &[Vec>], + ) { *layers = retry_operation(|| { let mut disk_state = Vec::new(); for i in u8::MIN..=u8::MAX { @@ -183,10 +222,16 @@ pub trait DbSimulator { } if let Some(layers) = Self::attempt_to_reset_model_to_disk_state(layers, &disk_state) { - Ok(layers) - } else { - Err(parity_db::Error::Corruption(format!("Not able to recover the database to one of the valid state. The current database state is: {:?}", disk_state))) + return Ok(layers) + } + for layers in old_layers { + if let Some(layers) = + Self::attempt_to_reset_model_to_disk_state(layers, &disk_state) + { + return Ok(layers) + } } + Err(parity_db::Error::Corruption(format!("Not able to recover the database to one of the valid state. The current database state is: {:?}", disk_state))) }) }