radicle: Configure database connections on open

Configuration of database connections is not performed on `open`, which
leaves room for error (e.g. to miss specifying configuration).

Methods on `Profile` automatically supply the configuration of the
profile.

In testing code, just using the default configuration suffices.
This commit is contained in:
Lorenz Leutgeb 2026-03-02 16:45:15 +01:00 committed by Fintan Halpenny
parent f3afe7b02a
commit 5aaf978f97
10 changed files with 108 additions and 57 deletions

View File

@ -20,6 +20,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
to create temporary patches.
- The `rad id` command will provide a better error message when a non-delegate
attempts to modify the identity document.
- The `journal_mode` and `synchronous` pragmas can now be configured using the
configuration file. The default values used are `WAL` and `NORMAL`, which
generates less I/O operations. On power loss, transactions might be rolled
back, but SQLite still guarantees consistency in this mode.
## Fixed Bugs

View File

@ -156,7 +156,7 @@ impl Environment {
db.migrate(radicle::cob::migrate::ignore).unwrap();
policy::Store::open(policies_db).unwrap();
home.database_mut()
home.database_mut(node::db::config::Config::default())
.unwrap()
.init(
&keypair.pk.into(),

View File

@ -175,18 +175,14 @@ impl Runtime {
.expect("Runtime::init: unable to solve proof-of-work puzzle");
log::info!(target: "node", "Opening node database..");
let db = home
.database_mut()?
.journal_mode(config.database.sqlite.pragma.journal_mode)?
.synchronous(config.database.sqlite.pragma.synchronous)?
.init(
&id,
announcement.features,
&announcement.alias,
&announcement.agent,
announcement.timestamp,
announcement.addresses.iter(),
)?;
let db = home.database_mut(config.database)?.init(
&id,
announcement.features,
&announcement.alias,
&announcement.agent,
announcement.timestamp,
announcement.addresses.iter(),
)?;
let mut stores: service::Stores<_> = db.clone().into();
if config.connect.is_empty() && stores.addresses().is_empty()? {

View File

@ -150,7 +150,11 @@ impl<G: Signer<Signature> + cyphernet::Ecdh> NodeHandle<G> {
pub fn routing(&self) -> impl Iterator<Item = (RepoId, NodeId)> {
use node::routing::Store as _;
self.home.routing_mut().unwrap().entries().unwrap()
self.home
.routing_mut(node::db::config::Config::default())
.unwrap()
.entries()
.unwrap()
}
pub fn inventory(&self) -> impl Iterator<Item = RepoId> + '_ {
@ -161,7 +165,11 @@ impl<G: Signer<Signature> + cyphernet::Ecdh> NodeHandle<G> {
/// Get sync status of a repo.
pub fn synced_seeds(&self, rid: &RepoId) -> Vec<node::seed::SyncedSeed> {
let db = Database::reader(self.home.node().join(node::NODE_DB_FILE)).unwrap();
let db = Database::reader(
self.home.node().join(node::NODE_DB_FILE),
node::db::config::Config::default(),
)
.unwrap();
let seeds = db.seeds_for(rid).unwrap();
seeds.into_iter().collect::<Result<Vec<_>, _>>().unwrap()
@ -446,7 +454,9 @@ impl Node<MockSigner> {
)
.unwrap();
let policies = home.policies_mut().unwrap();
let db = home.database_mut().unwrap();
let db = home
.database_mut(node::db::config::Config::default())
.unwrap();
let db = service::Stores::from(db);
log::debug!(target: "test", "Node::init {}: {}", config.alias, signer.public_key());

View File

@ -173,18 +173,21 @@ where
policies.seed(&repo.rid, Scope::Followed).unwrap();
}
// Initialize database.
let db = Database::open(config.tmp.path().join(node::NODE_DB_FILE))
.unwrap()
.init(
&id,
config.config.features(),
&config.config.alias,
&UserAgent::default(),
config.local_time.into(),
config.config.external_addresses.iter(),
)
.unwrap()
.into();
let db = Database::open(
config.tmp.path().join(node::NODE_DB_FILE),
node::db::config::Config::default(),
)
.unwrap()
.init(
&id,
config.config.features(),
&config.config.alias,
&UserAgent::default(),
config.local_time.into(),
config.config.external_addresses.iter(),
)
.unwrap()
.into();
let announcement =
service::gossip::node(&config.config, Timestamp::from(config.local_time) + 1);

View File

@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
from `FULL` to `NORMAL`, which generates less I/O operations. On power
loss, transactions might be rolled back, but SQLite still guarantees
consistency in this mode.
- Opening database connections requires specification of a configuration.
`radicle::Profile` conveniently provides methods that supply the
configuration from `radicle::Profile::config`.
### Removed

View File

@ -568,7 +568,7 @@ mod test {
fn test_empty() {
let tmp = tempfile::tempdir().unwrap();
let path = tmp.path().join("cache");
let cache = Database::open(path).unwrap();
let cache = Database::open(path, crate::node::db::config::Config::default()).unwrap();
assert!(cache.is_empty().unwrap());
}

View File

@ -82,19 +82,19 @@ impl Database {
const PRAGMA: &'static str = "PRAGMA foreign_keys = ON";
/// Open a database at the given path. Creates a new database if it
/// doesn't exist.
pub fn open<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
/// does not exist.
pub fn open<P: AsRef<Path>>(path: P, config: config::Config) -> Result<Self, Error> {
let mut db = sql::Connection::open_thread_safe(path)?;
db.set_busy_timeout(DB_WRITE_TIMEOUT.as_millis() as usize)?;
db.execute(Self::PRAGMA)?;
migrate(&db)?;
Ok(Self { db: Arc::new(db) })
Self { db: Arc::new(db) }.configure(config)
}
/// Same as [`Self::open`], but in read-only mode. This is useful to have multiple
/// open databases, as no locking is required.
pub fn reader<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
pub fn reader<P: AsRef<Path>>(path: P, config: config::Config) -> Result<Self, Error> {
let mut db = sql::Connection::open_thread_safe_with_flags(
path,
sql::OpenFlags::new().with_read_only(),
@ -102,7 +102,7 @@ impl Database {
db.set_busy_timeout(DB_READ_TIMEOUT.as_millis() as usize)?;
db.execute(Self::PRAGMA)?;
Ok(Self { db: Arc::new(db) })
Self { db: Arc::new(db) }.configure(config)
}
/// Set `journal_mode` pragma.
@ -163,6 +163,11 @@ impl Database {
pub fn bump(&self) -> Result<usize, Error> {
transaction(&self.db, bump)
}
fn configure(self, config: config::Config) -> Result<Self, Error> {
self.journal_mode(config.sqlite.pragma.journal_mode)?
.synchronous(config.sqlite.pragma.synchronous)
}
}
/// Get the `user_version` value from the database header.

View File

@ -265,7 +265,7 @@ mod test {
use crate::test::arbitrary;
fn database(path: &str) -> Database {
let db = Database::open(path).unwrap();
let db = Database::open(path, crate::node::db::config::Config::default()).unwrap();
// We don't want to test foreign key constraints here.
db.db.execute("PRAGMA foreign_keys = OFF").unwrap();

View File

@ -252,17 +252,14 @@ impl Profile {
// Create DBs.
home.policies_mut()?;
home.notifications_mut()?;
home.database_mut()?
.journal_mode(config.node.database.sqlite.pragma.journal_mode)?
.synchronous(config.node.database.sqlite.pragma.synchronous)?
.init(
&public_key,
config.node.features(),
&config.node.alias,
&UserAgent::default(),
LocalTime::now().into(),
config.node.external_addresses.iter(),
)?;
home.database_mut(config.node.database)?.init(
&public_key,
config.node.features(),
&config.node.alias,
&UserAgent::default(),
LocalTime::now().into(),
config.node.external_addresses.iter(),
)?;
// Migrate COBs cache.
let mut cobs = home.cobs_db_mut()?;
@ -364,7 +361,7 @@ impl Profile {
/// Return a multi-source store for aliases.
pub fn aliases(&self) -> Aliases {
let policies = self.home.policies().ok();
let db = self.home.database().ok();
let db = self.home.database(self.config.node.database).ok();
Aliases { policies, db }
}
@ -419,6 +416,24 @@ impl Profile {
Err(e) => Err(e.into()),
}
}
/// Return a handle to the database of the node, with SQLite configuration
/// from [`Self::config`] applied.
pub fn database_mut(&self) -> Result<node::Database, node::db::Error> {
self.home.database_mut(self.config.node.database)
}
/// Return a handle to a read-only database of the node, with SQLite
/// configuration from [`Self::config`] applied.
pub fn database(&self) -> Result<node::Database, node::db::Error> {
self.home.database(self.config.node.database)
}
/// Returns the routing store, with SQLite
/// configuration from [`Self::config`] applied.
pub fn routing(&self) -> Result<impl node::routing::Store, node::db::Error> {
self.home.routing(self.config.node.database)
}
}
impl std::ops::Deref for Profile {
@ -630,34 +645,49 @@ impl Home {
}
/// Return a handle to a read-only database of the node.
pub fn database(&self) -> Result<node::Database, node::db::Error> {
pub fn database(
&self,
config: node::db::config::Config,
) -> Result<node::Database, node::db::Error> {
let path = self.node().join(node::NODE_DB_FILE);
let db = node::Database::reader(path)?;
let db = node::Database::reader(path, config)?;
Ok(db)
}
/// Return a handle to the database of the node.
pub fn database_mut(&self) -> Result<node::Database, node::db::Error> {
pub fn database_mut(
&self,
config: node::db::config::Config,
) -> Result<node::Database, node::db::Error> {
let path = self.node().join(node::NODE_DB_FILE);
let db = node::Database::open(path)?;
let db = node::Database::open(path, config)?;
Ok(db)
}
/// Returns the address store.
pub fn addresses(&self) -> Result<impl node::address::Store, node::db::Error> {
self.database_mut()
pub fn addresses(
&self,
config: node::db::config::Config,
) -> Result<impl node::address::Store, node::db::Error> {
self.database_mut(config)
}
/// Returns the routing store.
pub fn routing(&self) -> Result<impl node::routing::Store, node::db::Error> {
self.database()
pub fn routing(
&self,
config: node::db::config::Config,
) -> Result<impl node::routing::Store, node::db::Error> {
self.database(config)
}
/// Returns the routing store, mutably.
pub fn routing_mut(&self) -> Result<impl node::routing::Store, node::db::Error> {
self.database_mut()
pub fn routing_mut(
&self,
config: node::db::config::Config,
) -> Result<impl node::routing::Store, node::db::Error> {
self.database_mut(config)
}
/// Get read access to the COBs cache.