diff --git a/CHANGELOG.md b/CHANGELOG.md index 12cecd497be..64a40040c2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] Description of the upcoming release here. +### Fixed +- [#1814](https://github.com/FuelLabs/fuel-core/pull/1814): Bugfix: the `iter_all_by_prefix` was not working for all tables. The change adds a `Rust` level filtering. +======= ### Added - [#1799](https://github.com/FuelLabs/fuel-core/pull/1799) Snapshot creation is now concurrent. diff --git a/crates/fuel-core/src/combined_database.rs b/crates/fuel-core/src/combined_database.rs index 788dee707b7..26a48468f20 100644 --- a/crates/fuel-core/src/combined_database.rs +++ b/crates/fuel-core/src/combined_database.rs @@ -151,14 +151,13 @@ impl CombinedDatabase { use fuel_core_chain_config::AddTable; use itertools::Itertools; let mut builder = StateConfigBuilder::default(); - use crate::database::IncludeAll; macro_rules! add_tables { ($($table: ty),*) => { $( let table = self .on_chain() - .entries::<$table>(IncludeAll, fuel_core_storage::iter::IterDirection::Forward) + .entries::<$table>(None, fuel_core_storage::iter::IterDirection::Forward) .try_collect()?; builder.add(table); )* diff --git a/crates/fuel-core/src/database.rs b/crates/fuel-core/src/database.rs index 2a274d63b52..7f6c3c823d5 100644 --- a/crates/fuel-core/src/database.rs +++ b/crates/fuel-core/src/database.rs @@ -104,42 +104,20 @@ impl Database { } } -pub trait KeyFilter { - fn start_at_prefix(&self) -> Option<&[u8]>; - fn should_stop(&self, key: &K) -> bool; -} - -pub struct IncludeAll; -impl KeyFilter for IncludeAll { - fn start_at_prefix(&self) -> Option<&[u8]> { - None - } - - fn should_stop(&self, _: &K) -> bool { - false - } -} - impl Database where DbDesc: DatabaseDescription, { pub fn entries<'a, T>( &'a self, - filter: impl KeyFilter + 'a, + prefix: Option>, direction: IterDirection, ) -> impl Iterator>> + 'a where T: TableWithBlueprint::Column> + 'a, T::Blueprint: BlueprintInspect, { - self.iter_all_filtered::(filter.start_at_prefix(), None, Some(direction)) - .take_while(move |result| { - let Ok((key, _)) = result.as_ref() else { - return true; - }; - !filter.should_stop(key) - }) + self.iter_all_filtered::(prefix, None, Some(direction)) .map_ok(|(key, value)| TableEntry { key, value }) } } @@ -1009,4 +987,48 @@ mod tests { ); } } + + #[cfg(feature = "rocksdb")] + #[test] + fn database_iter_all_by_prefix_works() { + use fuel_core_storage::tables::ContractsRawCode; + use fuel_core_types::fuel_types::ContractId; + use std::str::FromStr; + + let test = |mut db: Database| { + let contract_id_1 = ContractId::from_str( + "5962be5ebddc516cb4ed7d7e76365f59e0d231ac25b53f262119edf76564aab4", + ) + .unwrap(); + + let mut insert_empty_code = |id| { + StorageMutate::::insert(&mut db, &id, &[]).unwrap() + }; + insert_empty_code(contract_id_1); + + let contract_id_2 = ContractId::from_str( + "5baf0dcae7c114f647f6e71f1723f59bcfc14ecb28071e74895d97b14873c5dc", + ) + .unwrap(); + insert_empty_code(contract_id_2); + + let matched_keys: Vec<_> = db + .iter_all_by_prefix::(Some(contract_id_1)) + .map_ok(|(k, _)| k) + .try_collect() + .unwrap(); + + assert_eq!(matched_keys, vec![contract_id_1]); + }; + + let temp_dir = tempfile::tempdir().unwrap(); + let db = Database::::in_memory(); + // in memory passes + test(db); + + let db = Database::::open_rocksdb(temp_dir.path(), 1024 * 1024 * 1024) + .unwrap(); + // rocks db fails + test(db); + } } diff --git a/crates/fuel-core/src/service/genesis/exporter.rs b/crates/fuel-core/src/service/genesis/exporter.rs index 04054869c75..fd382d4e894 100644 --- a/crates/fuel-core/src/service/genesis/exporter.rs +++ b/crates/fuel-core/src/service/genesis/exporter.rs @@ -3,8 +3,6 @@ use crate::{ database::{ database_description::DatabaseDescription, Database, - IncludeAll, - KeyFilter, }, fuel_core_graphql_api::storage::transactions::{ OwnedTransactions, @@ -39,10 +37,7 @@ use itertools::Itertools; use tokio_util::sync::CancellationToken; -use self::filter::ByContractId; - use super::task_manager::TaskManager; -mod filter; pub struct Exporter { db: CombinedDatabase, @@ -74,7 +69,7 @@ where pub async fn write_full_snapshot(mut self) -> Result<(), anyhow::Error> { macro_rules! export { ($db: expr, $($table: ty),*) => { - $(self.spawn_task::<$table, _>(IncludeAll, $db)?;)* + $(self.spawn_task::<$table, _>(None, $db)?;)* }; } @@ -106,8 +101,7 @@ where ) -> Result<(), anyhow::Error> { macro_rules! export { ($($table: ty),*) => { - let filter = ByContractId::new(contract_id); - $(self.spawn_task::<$table, _>(filter, |ctx: &Self| ctx.db.on_chain())?;)* + $(self.spawn_task::<$table, _>(Some(contract_id.as_ref()), |ctx: &Self| ctx.db.on_chain())?;)* }; } export!( @@ -145,7 +139,7 @@ where fn spawn_task( &mut self, - filter: impl KeyFilter + Send + 'static, + prefix: Option<&[u8]>, db_picker: impl FnOnce(&Self) -> &Database, ) -> anyhow::Result<()> where @@ -160,9 +154,10 @@ where let group_size = self.group_size; let db = db_picker(self).clone(); + let prefix = prefix.map(|p| p.to_vec()); self.task_manager.spawn(move |cancel| { tokio_rayon::spawn(move || { - db.entries::(filter, IterDirection::Forward) + db.entries::(prefix, IterDirection::Forward) .chunks(group_size) .into_iter() .take_while(|_| !cancel.is_cancelled()) diff --git a/crates/fuel-core/src/service/genesis/exporter/filter.rs b/crates/fuel-core/src/service/genesis/exporter/filter.rs deleted file mode 100644 index ce30fd97ef0..00000000000 --- a/crates/fuel-core/src/service/genesis/exporter/filter.rs +++ /dev/null @@ -1,49 +0,0 @@ -use fuel_core_storage::{ - ContractsAssetKey, - ContractsStateKey, -}; -use fuel_core_types::fuel_types::ContractId; - -use crate::database::KeyFilter; - -#[derive(Debug, Clone, Copy)] -pub struct ByContractId { - contract_id: ContractId, -} - -impl ByContractId { - pub fn new(contract_id: ContractId) -> Self { - Self { contract_id } - } -} - -trait HasContractId { - fn extract_contract_id(&self) -> ContractId; -} - -impl KeyFilter for ByContractId { - fn start_at_prefix(&self) -> Option<&[u8]> { - Some(self.contract_id.as_ref()) - } - fn should_stop(&self, key: &K) -> bool { - key.extract_contract_id() != self.contract_id - } -} - -impl HasContractId for ContractId { - fn extract_contract_id(&self) -> ContractId { - *self - } -} - -impl HasContractId for ContractsAssetKey { - fn extract_contract_id(&self) -> ContractId { - *self.contract_id() - } -} - -impl HasContractId for ContractsStateKey { - fn extract_contract_id(&self) -> ContractId { - *self.contract_id() - } -} diff --git a/crates/fuel-core/src/state/rocks_db.rs b/crates/fuel-core/src/state/rocks_db.rs index 6d7dcc8e136..a51732c7171 100644 --- a/crates/fuel-core/src/state/rocks_db.rs +++ b/crates/fuel-core/src/state/rocks_db.rs @@ -470,10 +470,22 @@ where prefix, convert_to_rocksdb_direction(direction), ); + + // Setting prefix on the RocksDB level to optimize iteration. let mut opts = ReadOptions::default(); opts.set_prefix_same_as_start(true); - self._iter_all(column, opts, iter_mode).into_boxed() + let prefix = prefix.to_vec(); + self._iter_all(column, opts, iter_mode) + // Not all tables has a prefix set, so we need to filter out the keys. + .take_while(move |item| { + if let Ok((key, _)) = item { + key.starts_with(prefix.as_slice()) + } else { + true + } + }) + .into_boxed() } } (None, Some(start)) => {