Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion crates/db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod conv;
mod errors;
mod manager;

use std::num::NonZeroUsize;
use std::path::Path;

pub use conv::{DatabaseTypeConversionError, SqlTypeConvert};
Expand All @@ -22,8 +23,18 @@ pub struct Db {
impl Db {
/// Creates a new database instance with the provided connection pool.
pub fn new(database_filepath: &Path) -> Result<Self, DatabaseError> {
Self::new_with_pool_size(database_filepath, NonZeroUsize::new(16).expect("non-zero"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is somewhat unrelated, but I think the deadpool_diesel default of NUM_CPUS * 2 is a better default for the pool size than our hard-wired 16.

@Mirko-von-Leipzig Maybe we should just use that default instead of 16?

Obviously that'd mean some changes here: unless an explicit value is forced on the CLI we should not call builder.max_size(N) at all.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure that makes sense to me.

}

/// Creates a new database instance with a configurable SQLite connection pool size.
pub fn new_with_pool_size(
database_filepath: &Path,
sqlite_pool_size: NonZeroUsize,
) -> Result<Self, DatabaseError> {
let manager = ConnectionManager::new(database_filepath.to_str().unwrap());
let pool = deadpool_diesel::Pool::builder(manager).max_size(16).build()?;
let pool = deadpool_diesel::Pool::builder(manager)
.max_size(sqlite_pool_size.get())
.build()?;
Ok(Self { pool })
}

Expand Down
8 changes: 6 additions & 2 deletions crates/store/src/db/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::mem::size_of;
use std::num::NonZeroUsize;
use std::ops::{Deref, DerefMut, RangeInclusive};
use std::path::PathBuf;
use std::sync::Arc;
Expand Down Expand Up @@ -281,8 +282,11 @@ impl Db {

/// Open a connection to the DB and apply any pending migrations.
#[instrument(target = COMPONENT, skip_all)]
pub async fn load(database_filepath: PathBuf) -> Result<Self, DatabaseError> {
let db = miden_node_db::Db::new(&database_filepath)?;
pub async fn load(
database_filepath: PathBuf,
sqlite_pool_size: NonZeroUsize,
) -> Result<Self, DatabaseError> {
let db = miden_node_db::Db::new_with_pool_size(&database_filepath, sqlite_pool_size)?;
info!(
target: COMPONENT,
sqlite= %database_filepath.display(),
Expand Down
14 changes: 12 additions & 2 deletions crates/store/src/db/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1532,7 +1532,12 @@ async fn reconstruct_storage_map_from_db_pages_until_latest() {
let block2 = BlockNumber::from(2);
let block3 = BlockNumber::from(3);

let db = crate::db::Db::load(db_path).await.unwrap();
let db = crate::db::Db::load(
db_path,
miden_node_utils::clap::StorageOptions::default().sqlite_pool_size,
)
.await
.unwrap();
let slot_name_for_db = slot_name.clone();
db.query("insert paged values", move |db_conn| {
db_conn.transaction(|db_conn| {
Expand Down Expand Up @@ -1600,7 +1605,12 @@ async fn reconstruct_storage_map_from_db_returns_limit_exceeded_for_single_block

let block5 = BlockNumber::from(5);

let db = crate::db::Db::load(db_path).await.unwrap();
let db = crate::db::Db::load(
db_path,
miden_node_utils::clap::StorageOptions::default().sqlite_pool_size,
)
.await
.unwrap();
let slot_name_for_db = slot_name.clone();
db.query("insert entries in single block", move |db_conn| {
db_conn.transaction(|db_conn| {
Expand Down
2 changes: 1 addition & 1 deletion crates/store/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ impl State {
);

let database_filepath = data_directory.database_path();
let mut db = Db::load(database_filepath.clone())
let mut db = Db::load(database_filepath.clone(), storage_options.sqlite_pool_size)
.await
.map_err(StateInitializationError::DatabaseLoadError)?;

Expand Down
33 changes: 31 additions & 2 deletions crates/utils/src/clap.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Public module for share clap pieces to reduce duplication

use std::num::{NonZeroU32, NonZeroU64};
use std::num::{NonZeroU32, NonZeroU64, NonZeroUsize};
use std::time::Duration;

#[cfg(feature = "rocksdb")]
Expand All @@ -14,6 +14,7 @@ const DEFAULT_MAX_CONNECTION_AGE: Duration = Duration::from_mins(30);
const DEFAULT_REPLENISH_N_PER_SECOND_PER_IP: NonZeroU64 = NonZeroU64::new(16).unwrap();
const DEFAULT_BURST_SIZE: NonZeroU32 = NonZeroU32::new(128).unwrap();
const DEFAULT_MAX_CONCURRENT_CONNECTIONS: u64 = 1_000;
const DEFAULT_SQLITE_POOL_SIZE: NonZeroUsize = NonZeroUsize::new(16).unwrap();

// Formats a Duration into a human-readable string for display in clap help text
// and yields a &'static str by _leaking_ the string deliberately.
Expand Down Expand Up @@ -141,8 +142,17 @@ impl GrpcOptionsExternal {
/// Collection of per usage storage backend configurations.
///
/// Note: Currently only contains `rocksdb` related configuration.
#[derive(clap::Args, Clone, Debug, Default, PartialEq, Eq)]
#[derive(clap::Args, Clone, Debug, PartialEq, Eq)]
pub struct StorageOptions {
/// Maximum number of SQLite connections in the async pool.
#[arg(
long = "sqlite.pool_size",
env = "MIDEN_NODE_SQLITE_POOL_SIZE",
default_value_t = DEFAULT_SQLITE_POOL_SIZE,
value_name = "NUM"
)]
pub sqlite_pool_size: NonZeroUsize,

#[cfg(feature = "rocksdb")]
#[clap(flatten)]
pub account_tree: AccountTreeRocksDbOptions,
Expand Down Expand Up @@ -174,6 +184,7 @@ impl StorageOptions {
cache_size_in_bytes: DEFAULT_ROCKSDB_CACHE_SIZE,
};
Self {
sqlite_pool_size: DEFAULT_SQLITE_POOL_SIZE,
account_tree,
nullifier_tree,
account_state_forest,
Expand All @@ -183,3 +194,21 @@ impl StorageOptions {
Self::default()
}
}

impl Default for StorageOptions {
fn default() -> Self {
#[cfg(feature = "rocksdb")]
{
Self {
sqlite_pool_size: DEFAULT_SQLITE_POOL_SIZE,
account_tree: AccountTreeRocksDbOptions::default(),
nullifier_tree: NullifierTreeRocksDbOptions::default(),
account_state_forest: AccountStateForestRocksDbOptions::default(),
}
}
#[cfg(not(feature = "rocksdb"))]
{
Self { sqlite_pool_size: DEFAULT_SQLITE_POOL_SIZE }
}
}
}