From 8527dd87cc7a685b3ffd2054fca1a88574b93ea4 Mon Sep 17 00:00:00 2001 From: Zdenek Crha Date: Wed, 17 Jan 2024 21:20:34 +0100 Subject: [PATCH 1/4] convert_db: allow LMDB map size override --- src/garage/cli/convert_db.rs | 33 +++++++++++++++++++++++++++++---- 1 file changed, 29 insertions(+), 4 deletions(-) diff --git a/src/garage/cli/convert_db.rs b/src/garage/cli/convert_db.rs index 3c6ce69c..3b396c1b 100644 --- a/src/garage/cli/convert_db.rs +++ b/src/garage/cli/convert_db.rs @@ -22,16 +22,38 @@ pub struct ConvertDbOpt { /// Output database engine #[structopt(short = "b")] output_engine: String, + + #[structopt(flatten)] + output_open: OpenDbOpt, +} + +/// Open database config overrides +#[derive(StructOpt, Debug, Default)] +pub struct OpenDbOpt { + #[cfg(feature = "lmdb")] + #[structopt(flatten)] + lmdb: OpenLmdbOpt, +} + +/// Output LMDB database config overrides +#[cfg(feature = "lmdb")] +#[derive(StructOpt, Debug, Default)] +pub struct OpenLmdbOpt { + /// Output LMDB map size override + /// (supported suffixes: B, KiB, MiB, GiB, TiB, PiB) + #[cfg(feature = "lmdb")] + #[structopt(long = "lmdb-map-size", name = "bytes", display_order = 1_000)] + map_size: Option, } pub(crate) fn do_conversion(args: ConvertDbOpt) -> Result<()> { - let input = open_db(args.input_path, args.input_engine)?; - let output = open_db(args.output_path, args.output_engine)?; + let input = open_db(args.input_path, args.input_engine, OpenDbOpt::default())?; + let output = open_db(args.output_path, args.output_engine, args.output_open)?; output.import(&input)?; Ok(()) } -fn open_db(path: PathBuf, engine: String) -> Result { +fn open_db(path: PathBuf, engine: String, open: OpenDbOpt) -> Result { match engine.as_str() { #[cfg(feature = "sled")] "sled" => { @@ -51,7 +73,10 @@ fn open_db(path: PathBuf, engine: String) -> Result { Error(format!("Unable to create LMDB data directory: {}", e).into()) })?; - let map_size = lmdb_adapter::recommended_map_size(); + let map_size = match open.lmdb.map_size { + Some(c) => c.as_u64() as usize, + None => lmdb_adapter::recommended_map_size(), + }; let mut env_builder = lmdb_adapter::heed::EnvOpenOptions::new(); env_builder.max_dbs(100); From 4b54e053dfd1bd950e86ffad9b4155a47806d8f1 Mon Sep 17 00:00:00 2001 From: Zdenek Crha Date: Thu, 18 Jan 2024 17:57:53 +0100 Subject: [PATCH 2/4] convert_db: prevent conversion between same input/output engine Use optional DB open overrides for both input and output database. Duplicating the same override flag for input/output would result in too many, too long flags. It would be too costly for very rare edge-case where converting between same DB engine, just with different flags. Because overrides flags for different engines are disjoint and we are preventing conversion between same input/ouput DB engine, we can have only one set. The override flag will be passed either to input or output, based on engine type it belongs to. It will never be passed to both of them and cause unwelcome surprise to user. --- src/db/lib.rs | 47 ++++++++++++++++++++++++++++++++++++ src/garage/cli/convert_db.rs | 25 ++++++++++--------- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/src/db/lib.rs b/src/db/lib.rs index fe44b01e..427140f9 100644 --- a/src/db/lib.rs +++ b/src/db/lib.rs @@ -171,6 +171,53 @@ impl Db { } } +/// List of supported database engine types +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Engine { + #[cfg(feature = "lmdb")] + Lmdb, + + #[cfg(feature = "sqlite")] + Sqlite, + + #[cfg(feature = "sled")] + Sled, +} + +impl std::fmt::Display for Engine { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + #[cfg(feature = "lmdb")] + Self::Lmdb => fmt.write_str("lmdb"), + + #[cfg(feature = "sqlite")] + Self::Sqlite => fmt.write_str("sqlite"), + + #[cfg(feature = "sled")] + Self::Sled => fmt.write_str("sled"), + } + } +} + +impl std::str::FromStr for Engine { + type Err = Error; + + fn from_str(text: &str) -> Result { + match text { + #[cfg(feature = "lmdb")] + "lmdb" | "heed" => Ok(Self::Lmdb), + + #[cfg(feature = "sqlite")] + "sqlite" | "sqlite3" | "rusqlite" => Ok(Self::Sqlite), + + #[cfg(feature = "sled")] + "sled" => Ok(Self::Sled), + + kind => Err(Error(format!("Invalid DB engine: {}", kind).into())), + } + } +} + #[allow(clippy::len_without_is_empty)] impl Tree { #[inline] diff --git a/src/garage/cli/convert_db.rs b/src/garage/cli/convert_db.rs index 3b396c1b..d33010f4 100644 --- a/src/garage/cli/convert_db.rs +++ b/src/garage/cli/convert_db.rs @@ -14,14 +14,14 @@ pub struct ConvertDbOpt { /// Input database engine (sled, lmdb or sqlite; limited by db engines /// enabled in this build) #[structopt(short = "a")] - input_engine: String, + input_engine: Engine, /// Output database path #[structopt(short = "o")] output_path: PathBuf, /// Output database engine #[structopt(short = "b")] - output_engine: String, + output_engine: Engine, #[structopt(flatten)] output_open: OpenDbOpt, @@ -47,28 +47,32 @@ pub struct OpenLmdbOpt { } pub(crate) fn do_conversion(args: ConvertDbOpt) -> Result<()> { - let input = open_db(args.input_path, args.input_engine, OpenDbOpt::default())?; - let output = open_db(args.output_path, args.output_engine, args.output_open)?; + if args.input_engine == args.output_engine { + return Err(Error("input and output database engine must differ".into())); + } + + let input = open_db(args.input_path, args.input_engine, &args.output_open)?; + let output = open_db(args.output_path, args.output_engine, &args.output_open)?; output.import(&input)?; Ok(()) } -fn open_db(path: PathBuf, engine: String, open: OpenDbOpt) -> Result { - match engine.as_str() { +fn open_db(path: PathBuf, engine: Engine, open: &OpenDbOpt) -> Result { + match engine { #[cfg(feature = "sled")] - "sled" => { + Engine::Sled => { let db = sled_adapter::sled::Config::default().path(&path).open()?; Ok(sled_adapter::SledDb::init(db)) } #[cfg(feature = "sqlite")] - "sqlite" | "sqlite3" | "rusqlite" => { + Engine::Sqlite => { let db = sqlite_adapter::rusqlite::Connection::open(&path)?; db.pragma_update(None, "journal_mode", &"WAL")?; db.pragma_update(None, "synchronous", &"NORMAL")?; Ok(sqlite_adapter::SqliteDb::init(db)) } #[cfg(feature = "lmdb")] - "lmdb" | "heed" => { + Engine::Lmdb => { std::fs::create_dir_all(&path).map_err(|e| { Error(format!("Unable to create LMDB data directory: {}", e).into()) })?; @@ -87,8 +91,5 @@ fn open_db(path: PathBuf, engine: String, open: OpenDbOpt) -> Result { let db = env_builder.open(&path)?; Ok(lmdb_adapter::LmdbDb::init(db)) } - e => Err(Error( - format!("Invalid or unsupported DB engine: {}", e).into(), - )), } } From 74e72fc99690e24de498e42d87fc04a123f712d5 Mon Sep 17 00:00:00 2001 From: Zdenek Crha Date: Mon, 22 Jan 2024 17:52:39 +0100 Subject: [PATCH 3/4] convert_db: cleanup naming and comments for open overrides --- src/garage/cli/convert_db.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/garage/cli/convert_db.rs b/src/garage/cli/convert_db.rs index d33010f4..7307d1cf 100644 --- a/src/garage/cli/convert_db.rs +++ b/src/garage/cli/convert_db.rs @@ -24,10 +24,10 @@ pub struct ConvertDbOpt { output_engine: Engine, #[structopt(flatten)] - output_open: OpenDbOpt, + db_open: OpenDbOpt, } -/// Open database config overrides +/// Overrides for database open operation #[derive(StructOpt, Debug, Default)] pub struct OpenDbOpt { #[cfg(feature = "lmdb")] @@ -35,11 +35,11 @@ pub struct OpenDbOpt { lmdb: OpenLmdbOpt, } -/// Output LMDB database config overrides +/// Overrides for LMDB database open operation #[cfg(feature = "lmdb")] #[derive(StructOpt, Debug, Default)] pub struct OpenLmdbOpt { - /// Output LMDB map size override + /// LMDB map size override /// (supported suffixes: B, KiB, MiB, GiB, TiB, PiB) #[cfg(feature = "lmdb")] #[structopt(long = "lmdb-map-size", name = "bytes", display_order = 1_000)] @@ -51,8 +51,8 @@ pub(crate) fn do_conversion(args: ConvertDbOpt) -> Result<()> { return Err(Error("input and output database engine must differ".into())); } - let input = open_db(args.input_path, args.input_engine, &args.output_open)?; - let output = open_db(args.output_path, args.output_engine, &args.output_open)?; + let input = open_db(args.input_path, args.input_engine, &args.db_open)?; + let output = open_db(args.output_path, args.output_engine, &args.db_open)?; output.import(&input)?; Ok(()) } From 0eef8a69f0006de305281dd374cc63e7a46e4b80 Mon Sep 17 00:00:00 2001 From: Zdenek Crha Date: Mon, 22 Jan 2024 20:38:14 +0100 Subject: [PATCH 4/4] make all garage_db::Engine variants un-conditional Having all Engine enum variants conditional causes compilation errors when *none* of the DB engine features is enabled. This is not an issue for full garage build, but affects crates that use garage_db as dependency. Change all variants to be present at all times. It solves compilation errors and also allows us to better differentiate between invalid DB engine name and engine with support not compiled in current binary. --- src/db/lib.rs | 37 ++++++++++++++++-------------------- src/garage/cli/convert_db.rs | 8 ++++++++ 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/db/lib.rs b/src/db/lib.rs index 427140f9..eef3e177 100644 --- a/src/db/lib.rs +++ b/src/db/lib.rs @@ -172,30 +172,31 @@ impl Db { } /// List of supported database engine types +/// +/// The `enum` holds list of *all* database engines that are are be supported by crate, no matter +/// if relevant feature is enabled or not. It allows us to distinguish between invalid engine +/// and valid engine, whose support is not enabled via feature flag. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Engine { - #[cfg(feature = "lmdb")] Lmdb, - - #[cfg(feature = "sqlite")] Sqlite, - - #[cfg(feature = "sled")] Sled, } +impl Engine { + /// Return variant name as static `&str` + pub fn as_str(&self) -> &'static str { + match self { + Self::Lmdb => "lmdb", + Self::Sqlite => "sqlite", + Self::Sled => "sled", + } + } +} + impl std::fmt::Display for Engine { fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { - match self { - #[cfg(feature = "lmdb")] - Self::Lmdb => fmt.write_str("lmdb"), - - #[cfg(feature = "sqlite")] - Self::Sqlite => fmt.write_str("sqlite"), - - #[cfg(feature = "sled")] - Self::Sled => fmt.write_str("sled"), - } + self.as_str().fmt(fmt) } } @@ -204,15 +205,9 @@ impl std::str::FromStr for Engine { fn from_str(text: &str) -> Result { match text { - #[cfg(feature = "lmdb")] "lmdb" | "heed" => Ok(Self::Lmdb), - - #[cfg(feature = "sqlite")] "sqlite" | "sqlite3" | "rusqlite" => Ok(Self::Sqlite), - - #[cfg(feature = "sled")] "sled" => Ok(Self::Sled), - kind => Err(Error(format!("Invalid DB engine: {}", kind).into())), } } diff --git a/src/garage/cli/convert_db.rs b/src/garage/cli/convert_db.rs index 7307d1cf..044ccbb9 100644 --- a/src/garage/cli/convert_db.rs +++ b/src/garage/cli/convert_db.rs @@ -91,5 +91,13 @@ fn open_db(path: PathBuf, engine: Engine, open: &OpenDbOpt) -> Result { let db = env_builder.open(&path)?; Ok(lmdb_adapter::LmdbDb::init(db)) } + + // Pattern is unreachable when all supported DB engines are compiled into binary. The allow + // attribute is added so that we won't have to change this match in case stop building + // support for one or more engines by default. + #[allow(unreachable_patterns)] + engine => Err(Error( + format!("Engine support not available in this build: {}", engine).into(), + )), } }