From 7b6bf0820e25703eb1763f02adac08fa79edc8ce Mon Sep 17 00:00:00 2001 From: joshieDo <93316087+joshieDo@users.noreply.github.com> Date: Thu, 15 Dec 2022 15:33:49 +0800 Subject: [PATCH] fix(cli): remove usage of `StageDB` on `DbTool` (#448) * use view and update instead of StageDB * change DbTool docs * clippy --- bin/reth/src/db/mod.rs | 103 ++++++++++-------- crates/primitives/src/chain.rs | 4 +- crates/primitives/src/hex_bytes.rs | 12 +- crates/stages/src/stages/bodies.rs | 4 +- crates/storage/libmdbx-rs/benches/utils.rs | 4 +- .../storage/libmdbx-rs/tests/transaction.rs | 7 +- 6 files changed, 72 insertions(+), 62 deletions(-) diff --git a/bin/reth/src/db/mod.rs b/bin/reth/src/db/mod.rs index 69b8394185..73b4bb59fc 100644 --- a/bin/reth/src/db/mod.rs +++ b/bin/reth/src/db/mod.rs @@ -4,7 +4,7 @@ //! Database debugging tool use clap::{Parser, Subcommand}; -use eyre::Result; +use eyre::{Result, WrapErr}; use reth_db::{ cursor::{DbCursorRO, Walker}, database::Database, @@ -14,7 +14,6 @@ use reth_db::{ }; use reth_interfaces::test_utils::generators::random_block_range; use reth_provider::insert_canonical_block; -use reth_stages::StageDB; use std::path::Path; use tracing::info; @@ -71,35 +70,40 @@ impl Command { expanded_db_path, reth_db::mdbx::EnvKind::RW, )?; - db.create_tables()?; let mut tool = DbTool::new(&db)?; + match &self.command { // TODO: We'll need to add this on the DB trait. Subcommands::Stats { .. } => { - // Get the env from MDBX - let env = &tool.db.inner().inner; - let tx = env.begin_ro_txn()?; - for table in tables::TABLES.iter().map(|(_, name)| name) { - let table_db = tx.open_db(Some(table))?; - let stats = tx.db_stat(&table_db)?; + tool.db.view(|tx| { + for table in tables::TABLES.iter().map(|(_, name)| name) { + let table_db = + tx.inner.open_db(Some(table)).wrap_err("Could not open db.")?; - // Defaults to 16KB right now but we should - // re-evaluate depending on the DB we end up using - // (e.g. REDB does not have these options as configurable intentionally) - let page_size = stats.page_size() as usize; - let leaf_pages = stats.leaf_pages(); - let branch_pages = stats.branch_pages(); - let overflow_pages = stats.overflow_pages(); - let num_pages = leaf_pages + branch_pages + overflow_pages; - let table_size = page_size * num_pages; - tracing::info!( - "Table {} has {} entries (total size: {} KB)", - table, - stats.entries(), - table_size / 1024 - ); - } + let stats = tx + .inner + .db_stat(&table_db) + .wrap_err(format!("Could not find table: {table}"))?; + + // Defaults to 16KB right now but we should + // re-evaluate depending on the DB we end up using + // (e.g. REDB does not have these options as configurable intentionally) + let page_size = stats.page_size() as usize; + let leaf_pages = stats.leaf_pages(); + let branch_pages = stats.branch_pages(); + let overflow_pages = stats.overflow_pages(); + let num_pages = leaf_pages + branch_pages + overflow_pages; + let table_size = page_size * num_pages; + tracing::info!( + "Table {} has {} entries (total size: {} KB)", + table, + stats.entries(), + table_size / 1024 + ); + } + Ok::<(), eyre::Report>(()) + })??; } Subcommands::Seed { len } => { tool.seed(*len)?; @@ -113,28 +117,29 @@ impl Command { } } -/// Abstraction over StageDB for writing/reading from/to the DB -/// Wraps over the StageDB and derefs to a transaction. +/// Wrapper over DB that implements many useful DB queries. struct DbTool<'a, DB: Database> { - // TODO: StageDB derefs to Tx, is this weird or not? - pub(crate) db: StageDB<'a, DB>, + pub(crate) db: &'a DB, } impl<'a, DB: Database> DbTool<'a, DB> { - /// Takes a DB where the tables have already been created + /// Takes a DB where the tables have already been created. fn new(db: &'a DB) -> eyre::Result { - Ok(Self { db: StageDB::new(db)? }) + Ok(Self { db }) } /// Seeds the database with some random data, only used for testing fn seed(&mut self, len: u64) -> Result<()> { tracing::info!("generating random block range from 0 to {len}"); let chain = random_block_range(0..len, Default::default()); - chain.iter().try_for_each(|block| { - insert_canonical_block(&*self.db, block, true)?; - Ok::<_, eyre::Error>(()) - })?; - self.db.commit()?; + + self.db.update(|tx| { + chain.iter().try_for_each(|block| { + insert_canonical_block(tx, block, true)?; + Ok::<_, eyre::Error>(()) + }) + })??; + info!("Database committed with {len} blocks"); Ok(()) @@ -143,6 +148,9 @@ impl<'a, DB: Database> DbTool<'a, DB> { /// Lists the given table data fn list(&mut self, args: &ListArgs) -> Result<()> { match args.table.as_str() { + "canonical_headers" => { + self.list_table::(args.start, args.len)? + } "headers" => self.list_table::(args.start, args.len)?, "txs" => self.list_table::(args.start, args.len)?, _ => panic!(), @@ -151,18 +159,21 @@ impl<'a, DB: Database> DbTool<'a, DB> { } fn list_table(&mut self, start: usize, len: usize) -> Result<()> { - let mut cursor = self.db.cursor::()?; + let data = self.db.view(|tx| { + let mut cursor = tx.cursor::().expect("Was not able to obtain a cursor."); - // TODO: Upstream this in the DB trait. - let start_walker = cursor.current().transpose(); - let walker = Walker { - cursor: &mut cursor, - start: start_walker, - _tx_phantom: std::marker::PhantomData, - }; + // TODO: Upstream this in the DB trait. + let start_walker = cursor.current().transpose(); + let walker = Walker { + cursor: &mut cursor, + start: start_walker, + _tx_phantom: std::marker::PhantomData, + }; - let data = walker.skip(start).take(len).collect::>(); - dbg!(&data); + walker.skip(start).take(len).collect::>() + })?; + + println!("{data:?}"); Ok(()) } diff --git a/crates/primitives/src/chain.rs b/crates/primitives/src/chain.rs index 40a974e903..68b01d5626 100644 --- a/crates/primitives/src/chain.rs +++ b/crates/primitives/src/chain.rs @@ -175,13 +175,13 @@ mod tests { #[test] fn test_display_named_chain() { let chain = Chain::Named(ethers_core::types::Chain::Mainnet); - assert_eq!(format!("{}", chain), "mainnet"); + assert_eq!(format!("{chain}"), "mainnet"); } #[test] fn test_display_id_chain() { let chain = Chain::Id(1234); - assert_eq!(format!("{}", chain), "1234"); + assert_eq!(format!("{chain}"), "1234"); } #[test] diff --git a/crates/primitives/src/hex_bytes.rs b/crates/primitives/src/hex_bytes.rs index fa647048be..4efac16646 100644 --- a/crates/primitives/src/hex_bytes.rs +++ b/crates/primitives/src/hex_bytes.rs @@ -203,7 +203,7 @@ mod tests { fn test_from_bytes() { let b = bytes::Bytes::from("0123456789abcdef"); let wrapped_b = Bytes::from(b.clone()); - let expected = Bytes { 0: b }; + let expected = Bytes(b); assert_eq!(wrapped_b, expected); } @@ -212,7 +212,7 @@ mod tests { fn test_from_slice() { let arr = [1, 35, 69, 103, 137, 171, 205, 239]; let b = Bytes::from(&arr); - let expected = Bytes { 0: bytes::Bytes::from(arr.to_vec()) }; + let expected = Bytes(bytes::Bytes::from(arr.to_vec())); assert_eq!(b, expected); } @@ -221,8 +221,8 @@ mod tests { fn hex_formatting() { let b = Bytes::from(vec![1, 35, 69, 103, 137, 171, 205, 239]); let expected = String::from("0x0123456789abcdef"); - assert_eq!(format!("{:x}", b), expected); - assert_eq!(format!("{}", b), expected); + assert_eq!(format!("{b:x}"), expected); + assert_eq!(format!("{b}"), expected); } #[test] @@ -240,8 +240,8 @@ mod tests { #[test] fn test_debug_formatting() { let b = Bytes::from(vec![1, 35, 69, 103, 137, 171, 205, 239]); - assert_eq!(format!("{:?}", b), "Bytes(0x0123456789abcdef)"); - assert_eq!(format!("{:#?}", b), "Bytes(0x0123456789abcdef)"); + assert_eq!(format!("{b:?}"), "Bytes(0x0123456789abcdef)"); + assert_eq!(format!("{b:#?}"), "Bytes(0x0123456789abcdef)"); } #[test] diff --git a/crates/stages/src/stages/bodies.rs b/crates/stages/src/stages/bodies.rs index 592e9f3da9..ba0d94b573 100644 --- a/crates/stages/src/stages/bodies.rs +++ b/crates/stages/src/stages/bodies.rs @@ -682,10 +682,10 @@ mod tests { Some((key, _)) => key, None => return Ok(()), }; - let mut walker = tx_count_cursor.walk(first_tx_count_key)?.peekable(); + let walker = tx_count_cursor.walk(first_tx_count_key)?.peekable(); let mut prev_entry: Option<(BlockNumHash, NumTransactions)> = None; - while let Some(entry) = walker.next() { + for entry in walker { let (key, count) = entry?; // Validate sequentiality only after prev progress, diff --git a/crates/storage/libmdbx-rs/benches/utils.rs b/crates/storage/libmdbx-rs/benches/utils.rs index b1a030b495..ea353b3e49 100644 --- a/crates/storage/libmdbx-rs/benches/utils.rs +++ b/crates/storage/libmdbx-rs/benches/utils.rs @@ -2,11 +2,11 @@ use reth_libmdbx::{Environment, NoWriteMap, WriteFlags}; use tempfile::{tempdir, TempDir}; pub fn get_key(n: u32) -> String { - format!("key{}", n) + format!("key{n}") } pub fn get_data(n: u32) -> String { - format!("data{}", n) + format!("data{n}") } pub fn setup_bench_db(num_rows: u32) -> (TempDir, Environment) { diff --git a/crates/storage/libmdbx-rs/tests/transaction.rs b/crates/storage/libmdbx-rs/tests/transaction.rs index 738f5f2286..53f1c54ef7 100644 --- a/crates/storage/libmdbx-rs/tests/transaction.rs +++ b/crates/storage/libmdbx-rs/tests/transaction.rs @@ -261,8 +261,7 @@ fn test_concurrent_writers() { threads.push(thread::spawn(move || { let txn = writer_env.begin_rw_txn().unwrap(); let db = txn.open_db(None).unwrap(); - txn.put(&db, format!("{}{}", key, i), format!("{}{}", val, i), WriteFlags::empty()) - .unwrap(); + txn.put(&db, format!("{key}{i}"), format!("{val}{i}"), WriteFlags::empty()).unwrap(); txn.commit().is_ok() })); } @@ -273,8 +272,8 @@ fn test_concurrent_writers() { for i in 0..n { assert_eq!( - Cow::>::Owned(format!("{}{}", val, i).into_bytes()), - txn.get(&db, format!("{}{}", key, i).as_bytes()).unwrap().unwrap() + Cow::>::Owned(format!("{val}{i}").into_bytes()), + txn.get(&db, format!("{key}{i}").as_bytes()).unwrap().unwrap() ); } }