From aa5b12af44cfbcb765c540888a23cc48f5de896c Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Tue, 27 Jan 2026 23:06:41 +0100 Subject: [PATCH] refactor(db): make Tx::inner field private with accessor (#21490) Co-authored-by: Amp --- crates/cli/commands/src/db/list.rs | 4 +- crates/cli/commands/src/db/stats.rs | 8 +-- crates/storage/db/benches/criterion.rs | 12 +++-- crates/storage/db/benches/hash_keys.rs | 52 +++++++------------ crates/storage/db/benches/utils.rs | 4 +- .../storage/db/src/implementation/mdbx/mod.rs | 5 +- .../storage/db/src/implementation/mdbx/tx.rs | 7 ++- 7 files changed, 43 insertions(+), 49 deletions(-) diff --git a/crates/cli/commands/src/db/list.rs b/crates/cli/commands/src/db/list.rs index 8e3db03fb7..eee500eb53 100644 --- a/crates/cli/commands/src/db/list.rs +++ b/crates/cli/commands/src/db/list.rs @@ -101,8 +101,8 @@ impl TableViewer<()> for ListTableViewer<'_, N> { // We may be using the tui for a long time tx.disable_long_read_transaction_safety(); - let table_db = tx.inner.open_db(Some(self.args.table.name())).wrap_err("Could not open db.")?; - let stats = tx.inner.db_stat(table_db.dbi()).wrap_err(format!("Could not find table: {}", self.args.table.name()))?; + let table_db = tx.inner().open_db(Some(self.args.table.name())).wrap_err("Could not open db.")?; + let stats = tx.inner().db_stat(table_db.dbi()).wrap_err(format!("Could not find table: {}", self.args.table.name()))?; let total_entries = stats.entries(); let final_entry_idx = total_entries.saturating_sub(1); if self.args.skip > final_entry_idx { diff --git a/crates/cli/commands/src/db/stats.rs b/crates/cli/commands/src/db/stats.rs index 62c8af1f40..85edbeeb4e 100644 --- a/crates/cli/commands/src/db/stats.rs +++ b/crates/cli/commands/src/db/stats.rs @@ -92,10 +92,10 @@ impl Command { db_tables.sort(); let mut total_size = 0; for db_table in db_tables { - let table_db = tx.inner.open_db(Some(db_table)).wrap_err("Could not open db.")?; + let table_db = tx.inner().open_db(Some(db_table)).wrap_err("Could not open db.")?; let stats = tx - .inner + .inner() .db_stat(table_db.dbi()) .wrap_err(format!("Could not find table: {db_table}"))?; @@ -136,9 +136,9 @@ impl Command { .add_cell(Cell::new(human_bytes(total_size as f64))); table.add_row(row); - let freelist = tx.inner.env().freelist()?; + let freelist = tx.inner().env().freelist()?; let pagesize = - tx.inner.db_stat(mdbx::Database::freelist_db().dbi())?.page_size() as usize; + tx.inner().db_stat(mdbx::Database::freelist_db().dbi())?.page_size() as usize; let freelist_size = freelist * pagesize; let mut row = Row::new(); diff --git a/crates/storage/db/benches/criterion.rs b/crates/storage/db/benches/criterion.rs index 7d62384c16..9d667e601d 100644 --- a/crates/storage/db/benches/criterion.rs +++ b/crates/storage/db/benches/criterion.rs @@ -137,7 +137,8 @@ where for (k, _, v, _) in input { crsr.append(k, &v).expect("submit"); } - tx.inner.commit().unwrap() + drop(crsr); + tx.commit().unwrap() }, ) }); @@ -157,8 +158,8 @@ where let (k, _, v, _) = input.get(index).unwrap().clone(); crsr.insert(k, &v).expect("submit"); } - - tx.inner.commit().unwrap() + drop(crsr); + tx.commit().unwrap() }, ) }); @@ -219,7 +220,8 @@ where for (k, _, v, _) in input { crsr.append_dup(k, v).expect("submit"); } - tx.inner.commit().unwrap() + drop(crsr); + tx.commit().unwrap() }, ) }); @@ -239,7 +241,7 @@ where let (k, _, v, _) = input.get(index).unwrap().clone(); tx.put::(k, v).unwrap(); } - tx.inner.commit().unwrap(); + tx.commit().unwrap() }, ) }); diff --git a/crates/storage/db/benches/hash_keys.rs b/crates/storage/db/benches/hash_keys.rs index b55965e1e7..ed21213be5 100644 --- a/crates/storage/db/benches/hash_keys.rs +++ b/crates/storage/db/benches/hash_keys.rs @@ -16,10 +16,9 @@ use reth_db_api::{ cursor::DbCursorRW, database::Database, table::{Table, TableRow}, - transaction::DbTxMut, + transaction::{DbTx, DbTxMut}, }; use reth_fs_util as fs; -use std::hint::black_box; mod utils; use utils::*; @@ -178,17 +177,13 @@ fn append(db: DatabaseEnv, input: Vec<(::Key, ::Value where T: Table, { - { - let tx = db.tx_mut().expect("tx"); - let mut crsr = tx.cursor_write::().expect("cursor"); - black_box({ - for (k, v) in input { - crsr.append(k, &v).expect("submit"); - } - - tx.inner.commit().unwrap() - }); + let tx = db.tx_mut().expect("tx"); + let mut crsr = tx.cursor_write::().expect("cursor"); + for (k, v) in input { + crsr.append(k, &v).expect("submit"); } + drop(crsr); + tx.commit().unwrap(); db } @@ -196,17 +191,13 @@ fn insert(db: DatabaseEnv, input: Vec<(::Key, ::Value where T: Table, { - { - let tx = db.tx_mut().expect("tx"); - let mut crsr = tx.cursor_write::().expect("cursor"); - black_box({ - for (k, v) in input { - crsr.insert(k, &v).expect("submit"); - } - - tx.inner.commit().unwrap() - }); + let tx = db.tx_mut().expect("tx"); + let mut crsr = tx.cursor_write::().expect("cursor"); + for (k, v) in input { + crsr.insert(k, &v).expect("submit"); } + drop(crsr); + tx.commit().unwrap(); db } @@ -214,16 +205,11 @@ fn put(db: DatabaseEnv, input: Vec<(::Key, ::Value)>) where T: Table, { - { - let tx = db.tx_mut().expect("tx"); - black_box({ - for (k, v) in input { - tx.put::(k, v).expect("submit"); - } - - tx.inner.commit().unwrap() - }); + let tx = db.tx_mut().expect("tx"); + for (k, v) in input { + tx.put::(k, v).expect("submit"); } + tx.commit().unwrap(); db } @@ -243,11 +229,11 @@ where T: Table, { db.view(|tx| { - let table_db = tx.inner.open_db(Some(T::NAME)).map_err(|_| "Could not open db.").unwrap(); + let table_db = tx.inner().open_db(Some(T::NAME)).map_err(|_| "Could not open db.").unwrap(); println!( "{:?}\n", - tx.inner + tx.inner() .db_stat(table_db.dbi()) .map_err(|_| format!("Could not find table: {}", T::NAME)) .map(|stats| { diff --git a/crates/storage/db/benches/utils.rs b/crates/storage/db/benches/utils.rs index 8c430342e7..5abad07c4d 100644 --- a/crates/storage/db/benches/utils.rs +++ b/crates/storage/db/benches/utils.rs @@ -5,7 +5,7 @@ use alloy_primitives::Bytes; use reth_db::{test_utils::create_test_rw_db_with_path, DatabaseEnv}; use reth_db_api::{ table::{Compress, Encode, Table, TableRow}, - transaction::DbTxMut, + transaction::{DbTx, DbTxMut}, Database, }; use reth_fs_util as fs; @@ -68,7 +68,7 @@ where for (k, _, v, _) in pair.clone() { tx.put::(k, v).expect("submit"); } - tx.inner.commit().unwrap(); + tx.commit().unwrap(); } db.into_inner_db() diff --git a/crates/storage/db/src/implementation/mdbx/mod.rs b/crates/storage/db/src/implementation/mdbx/mod.rs index 07b09b3ef5..8a95115eee 100644 --- a/crates/storage/db/src/implementation/mdbx/mod.rs +++ b/crates/storage/db/src/implementation/mdbx/mod.rs @@ -274,10 +274,11 @@ impl DatabaseMetrics for DatabaseEnv { let _ = self .view(|tx| { for table in Tables::ALL.iter().map(Tables::name) { - let table_db = tx.inner.open_db(Some(table)).wrap_err("Could not open db.")?; + let table_db = + tx.inner().open_db(Some(table)).wrap_err("Could not open db.")?; let stats = tx - .inner + .inner() .db_stat(table_db.dbi()) .wrap_err(format!("Could not find table: {table}"))?; diff --git a/crates/storage/db/src/implementation/mdbx/tx.rs b/crates/storage/db/src/implementation/mdbx/tx.rs index c693c5a80d..5b4acad700 100644 --- a/crates/storage/db/src/implementation/mdbx/tx.rs +++ b/crates/storage/db/src/implementation/mdbx/tx.rs @@ -30,7 +30,7 @@ const LONG_TRANSACTION_DURATION: Duration = Duration::from_secs(60); #[derive(Debug)] pub struct Tx { /// Libmdbx-sys transaction. - pub inner: Transaction, + inner: Transaction, /// Cached MDBX DBIs for reuse. dbis: Arc>, @@ -62,6 +62,11 @@ impl Tx { Ok(Self { inner, dbis, metrics_handler }) } + /// Returns a reference to the inner libmdbx transaction. + pub const fn inner(&self) -> &Transaction { + &self.inner + } + /// Gets this transaction ID. pub fn id(&self) -> reth_libmdbx::Result { self.metrics_handler.as_ref().map_or_else(|| self.inner.id(), |handler| Ok(handler.txn_id))