From b886e9be47e748d6ffa54d4cf516cdd8311e9fa1 Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Mon, 26 Oct 2026 18:12:24 -0700 Subject: [PATCH 1/2] feat!: let the engine control SystemConfig defaults Stop pinning a copy of every engine default in this binding. SystemConfig fields are now Option, defaulting to None meaning "use the engine default". Only explicitly set fields are forwarded: numerics via the existing sentinels, booleans via a new -1/0/1 tristate in new_database. Unset settings follow engine-side default changes automatically (e.g. LadybugDB/ladybug#1435's enable_multi_writes flip needs no binding change). Explicitly passed values behave exactly as before. Note: this also retires two stale Rust-side defaults that disagreed with the engine: max_db_size (was 4 GiB, now engine default) and throw_on_wal_replay_failure (was true, now engine default false). BREAKING CHANGE: SystemConfig fields changed from concrete types to Option; the new_database FFI booleans are now i64 tristates. --- include/lbug_rs.h | 7 +-- src/database.rs | 128 ++++++++++++++++++++++------------------------ src/ffi.rs | 17 +++--- src/lbug_rs.cpp | 31 +++++++---- src/lib.rs | 2 +- 5 files changed, 99 insertions(+), 86 deletions(-) diff --git a/include/lbug_rs.h b/include/lbug_rs.h index 84b7fd7..a8d83ac 100644 --- a/include/lbug_rs.h +++ b/include/lbug_rs.h @@ -115,10 +115,11 @@ inline lbug::common::LogicalTypeID logical_type_get_logical_type_id( } /* Database */ +// Boolean settings use a tristate: negative keeps the engine default. std::unique_ptr new_database(std::string_view databasePath, - uint64_t bufferPoolSize, uint64_t maxNumThreads, bool enableCompression, bool readOnly, - uint64_t maxDBSize, bool autoCheckpoint, int64_t checkpointThreshold, - bool throwOnWalReplayFailure, bool enableChecksums, bool enableMultiWrites); + uint64_t bufferPoolSize, uint64_t maxNumThreads, int64_t enableCompression, int64_t readOnly, + uint64_t maxDBSize, int64_t autoCheckpoint, int64_t checkpointThreshold, + int64_t throwOnWalReplayFailure, int64_t enableChecksums, int64_t enableMultiWrites); void database_set_logging_level(lbug::main::Database& database, const std::string& level); diff --git a/src/database.rs b/src/database.rs index 82a9ff4..f2bd916 100644 --- a/src/database.rs +++ b/src/database.rs @@ -14,113 +14,109 @@ pub struct Database { unsafe impl Send for Database {} unsafe impl Sync for Database {} -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] /// Configuration options for the database. +/// +/// Every field defaults to `None`, which means "use the engine default". +/// Only explicitly set fields override the engine configuration, so +/// engine-side default changes apply automatically without a binding update. pub struct SystemConfig { /// Max size of the buffer pool in bytes /// /// The larger the buffer pool, the more data from the database files is kept in memory, /// reducing the amount of File I/O. /// - /// Defaults to an auto-detected size - buffer_pool_size: u64, - /// The maximum number of threads to use when executing queries - /// Defaults to auto-detect from the number of processors - max_num_threads: u64, - /// When true, new columns will be compressed if possible - /// Defaults to true - enable_compression: bool, - /// Whether or not the database can be written to from this instance - /// Defaults to not read-only - read_only: bool, - /// The maximum size of the database. Defaults to 8TB - max_db_size: u64, + /// If None (default), the engine default (auto-detected size) is used. + buffer_pool_size: Option, + /// The maximum number of threads to use when executing queries. + /// If None (default), the engine default (auto-detected) is used. + max_num_threads: Option, + /// When true, new columns will be compressed if possible. + /// If None (default), the engine default is used. + enable_compression: Option, + /// Whether or not the database can be written to from this instance. + /// If None (default), the engine default (writable) is used. + read_only: Option, + /// The maximum size of the database. + /// If None (default), the engine default is used. + max_db_size: Option, /// If true, the database will automatically checkpoint when the size of the WAL file exceeds the checkpoint threshold. - auto_checkpoint: bool, + /// If None (default), the engine default is used. + auto_checkpoint: Option, /// The threshold of the WAL file size in bytes. When the size of the WAL file exceeds this threshold, the database will checkpoint if autoCheckpoint is true. - checkpoint_threshold: i64, + /// If None (default), the engine default is used. + checkpoint_threshold: Option, /// If true, any WAL replaying failure when loading the database will throw an error. Otherwise, Lbug will silently ignore the failure and replay up to where the error occured. - throw_on_wal_replay_failure: bool, + /// If None (default), the engine default is used. + throw_on_wal_replay_failure: Option, /// If true, the database will use checksums to detect corruption in the WAL file. - enable_checksums: bool, + /// If None (default), the engine default is used. + enable_checksums: Option, /// If true, multiple concurrent write transactions are allowed. - /// Defaults to false - enable_multi_writes: bool, + /// If None (default), the engine default is used. + enable_multi_writes: Option, } #[cfg(test)] pub(crate) const SYSTEM_CONFIG_FOR_TESTS: SystemConfig = SystemConfig { - buffer_pool_size: 0, - max_num_threads: 0, - enable_compression: true, - read_only: false, + buffer_pool_size: None, + max_num_threads: None, + enable_compression: None, + read_only: None, // Use a smaller max DB size since these tests don't need a large DB and it limits the number of // databases which can be open in a single process - max_db_size: 16 * 1024 * 1024 * 1024, - auto_checkpoint: true, - checkpoint_threshold: -1_i64, - throw_on_wal_replay_failure: true, - enable_checksums: true, - enable_multi_writes: false, + max_db_size: Some(16 * 1024 * 1024 * 1024), + auto_checkpoint: None, + checkpoint_threshold: None, + throw_on_wal_replay_failure: None, + enable_checksums: None, + enable_multi_writes: None, }; -impl Default for SystemConfig { - fn default() -> SystemConfig { - SystemConfig { - buffer_pool_size: 0, - max_num_threads: 0, - enable_compression: true, - read_only: false, - // This is a little weird, but it's a temporary interface - max_db_size: u64::from(u32::MAX), - auto_checkpoint: true, - checkpoint_threshold: -1_i64, - throw_on_wal_replay_failure: true, - enable_checksums: true, - enable_multi_writes: false, - } - } +/// Maps an optional boolean to the FFI tristate: -1 keeps the engine default. +fn bool_or_default(value: Option) -> i64 { + value.map_or(-1, i64::from) } impl SystemConfig { pub fn buffer_pool_size(mut self, buffer_pool_size: u64) -> Self { - self.buffer_pool_size = buffer_pool_size; + self.buffer_pool_size = Some(buffer_pool_size); self } pub fn max_num_threads(mut self, max_num_threads: u64) -> Self { - self.max_num_threads = max_num_threads; + self.max_num_threads = Some(max_num_threads); self } pub fn enable_compression(mut self, enable_compression: bool) -> Self { - self.enable_compression = enable_compression; + self.enable_compression = Some(enable_compression); self } pub fn read_only(mut self, read_only: bool) -> Self { - self.read_only = read_only; + self.read_only = Some(read_only); self } pub fn max_db_size(mut self, max_db_size: u64) -> Self { - self.max_db_size = max_db_size; + self.max_db_size = Some(max_db_size); self } pub fn auto_checkpoint(mut self, auto_checkpoint: bool) -> Self { - self.auto_checkpoint = auto_checkpoint; + self.auto_checkpoint = Some(auto_checkpoint); self } pub fn checkpoint_threshold(mut self, checkpoint_threshold: i64) -> Self { - self.checkpoint_threshold = checkpoint_threshold; + self.checkpoint_threshold = Some(checkpoint_threshold); self } pub fn throw_on_wal_replay_failure(mut self, throw_on_wal_replay_failure: bool) -> Self { - self.throw_on_wal_replay_failure = throw_on_wal_replay_failure; + self.throw_on_wal_replay_failure = Some(throw_on_wal_replay_failure); self } pub fn enable_checksums(mut self, enable_checksums: bool) -> Self { - self.enable_checksums = enable_checksums; + self.enable_checksums = Some(enable_checksums); self } pub fn enable_multi_writes(mut self, enable_multi_writes: bool) -> Self { - self.enable_multi_writes = enable_multi_writes; + self.enable_multi_writes = Some(enable_multi_writes); self } } @@ -138,16 +134,16 @@ impl Database { Ok(Database { db: UnsafeCell::new(ffi::new_database( ffi::StringView::new(&path.as_ref().display().to_string()), - config.buffer_pool_size, - config.max_num_threads, - config.enable_compression, - config.read_only, - config.max_db_size, - config.auto_checkpoint, - config.checkpoint_threshold, - config.throw_on_wal_replay_failure, - config.enable_checksums, - config.enable_multi_writes, + config.buffer_pool_size.unwrap_or(0), + config.max_num_threads.unwrap_or(0), + bool_or_default(config.enable_compression), + bool_or_default(config.read_only), + config.max_db_size.unwrap_or(u64::MAX), + bool_or_default(config.auto_checkpoint), + config.checkpoint_threshold.unwrap_or(-1), + bool_or_default(config.throw_on_wal_replay_failure), + bool_or_default(config.enable_checksums), + bool_or_default(config.enable_multi_writes), )?), }) } diff --git a/src/ffi.rs b/src/ffi.rs index 1c97595..1634ab4 100644 --- a/src/ffi.rs +++ b/src/ffi.rs @@ -157,19 +157,22 @@ pub(crate) mod ffi { #[namespace = "lbug::main"] type Database; - #[allow(clippy::fn_params_excessive_bools)] + // Boolean settings use a tristate: -1 keeps the engine default, + // 0/1 set it explicitly. Unset numerics use the existing sentinels + // (0 for sizes resolved by the engine, u64::MAX for maxDBSize, + // -1 for checkpoint_threshold). fn new_database( databasePath: StringView, bufferPoolSize: u64, maxNumThreads: u64, - enableCompression: bool, - readOnly: bool, + enableCompression: i64, + readOnly: i64, maxDBSize: u64, - auto_checkpoint: bool, + auto_checkpoint: i64, checkpoint_threshold: i64, - throw_on_wal_replay_failure: bool, - enable_checksums: bool, - enableMultiWrites: bool, + throw_on_wal_replay_failure: i64, + enable_checksums: i64, + enableMultiWrites: i64, ) -> Result>; } diff --git a/src/lbug_rs.cpp b/src/lbug_rs.cpp index b3d7c7f..7880ba8 100644 --- a/src/lbug_rs.cpp +++ b/src/lbug_rs.cpp @@ -72,9 +72,10 @@ std::unique_ptr> logical_type_get_struct_ } std::unique_ptr new_database(std::string_view databasePath, uint64_t bufferPoolSize, - uint64_t maxNumThreads, bool enableCompression, bool readOnly, uint64_t maxDBSize, - bool autoCheckpoint, int64_t checkpointThreshold, bool throwOnWalReplayFailure, - bool enableChecksums, bool enableMultiWrites) { + uint64_t maxNumThreads, int64_t enableCompression, int64_t readOnly, uint64_t maxDBSize, + int64_t autoCheckpoint, int64_t checkpointThreshold, int64_t throwOnWalReplayFailure, + int64_t enableChecksums, int64_t enableMultiWrites) { + // Boolean settings use a tristate: negative keeps the engine default. auto systemConfig = SystemConfig(); if (bufferPoolSize > 0) { systemConfig.bufferPoolSize = bufferPoolSize; @@ -82,18 +83,30 @@ std::unique_ptr new_database(std::string_view databasePath, uint64_t b if (maxNumThreads > 0) { systemConfig.maxNumThreads = maxNumThreads; } - systemConfig.readOnly = readOnly; - systemConfig.enableCompression = enableCompression; + if (enableCompression >= 0) { + systemConfig.enableCompression = enableCompression != 0; + } + if (readOnly >= 0) { + systemConfig.readOnly = readOnly != 0; + } if (maxDBSize != -1u) { systemConfig.maxDBSize = maxDBSize; } - systemConfig.autoCheckpoint = autoCheckpoint; + if (autoCheckpoint >= 0) { + systemConfig.autoCheckpoint = autoCheckpoint != 0; + } if (checkpointThreshold >= 0) { systemConfig.checkpointThreshold = checkpointThreshold; } - systemConfig.throwOnWalReplayFailure = throwOnWalReplayFailure; - systemConfig.enableChecksums = enableChecksums; - systemConfig.enableMultiWrites = enableMultiWrites; + if (throwOnWalReplayFailure >= 0) { + systemConfig.throwOnWalReplayFailure = throwOnWalReplayFailure != 0; + } + if (enableChecksums >= 0) { + systemConfig.enableChecksums = enableChecksums != 0; + } + if (enableMultiWrites >= 0) { + systemConfig.enableMultiWrites = enableMultiWrites != 0; + } return std::make_unique(databasePath, systemConfig); } diff --git a/src/lib.rs b/src/lib.rs index 2cc00ed..312a8b7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -78,7 +78,7 @@ //! //! ## Memory //! -//! [`SystemConfig::default`] leaves `buffer_pool_size` at 0, which the engine resolves from the +//! [`SystemConfig::default`] leaves `buffer_pool_size` unset, which the engine resolves from the //! host's physical memory, so an embedding process reserves a large share of RAM regardless of //! the database's size (about 6 GB resident on a 15 GiB host for a 200,000-edge graph). Set //! [`SystemConfig::buffer_pool_size`] explicitly when the process shares the machine. From bbdc9cdb795ab7eec2812bf2599701c14ed23b9f Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Mon, 26 Oct 2026 19:32:26 -0700 Subject: [PATCH 2/2] fix: engine-default follow-ups for checksums test and max_db_size sentinel The checksums test relied on the binding pinning throw_on_wal_replay_failure=true; opt into throwing explicitly since unset settings now follow the engine default (false). Also fix a latent -1u comparison bug in new_database: -1u is a 32-bit unsigned int, so comparing the uint64_t maxDBSize against it wrongly treated UINT64_MAX (the keep-engine-default sentinel) as an explicit value, failing with 'max db size should be a power of 2'. The old Rust default (u32::MAX) accidentally matched the buggy comparison. --- src/database.rs | 7 ++++++- src/lbug_rs.cpp | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/database.rs b/src/database.rs index f2bd916..8bcf488 100644 --- a/src/database.rs +++ b/src/database.rs @@ -310,9 +310,14 @@ mod tests { conn.query("create node table testtest1(id int64 primary key)")?; } { + // WAL replay failures must be configured to throw: the engine + // default (which this binding now follows) is to silently ignore + // them. Database::new( db_path.clone(), - SYSTEM_CONFIG_FOR_TESTS.enable_checksums(false), + SYSTEM_CONFIG_FOR_TESTS + .enable_checksums(false) + .throw_on_wal_replay_failure(true), ) .expect_err("An error should be thrown if the enable_checksums config doesn't match the one used to produce a WAL file"); } diff --git a/src/lbug_rs.cpp b/src/lbug_rs.cpp index 7880ba8..00f1751 100644 --- a/src/lbug_rs.cpp +++ b/src/lbug_rs.cpp @@ -1,5 +1,6 @@ #include "lbug_rs.h" +#include #include using lbug::common::ArrayTypeInfo; @@ -89,7 +90,11 @@ std::unique_ptr new_database(std::string_view databasePath, uint64_t b if (readOnly >= 0) { systemConfig.readOnly = readOnly != 0; } - if (maxDBSize != -1u) { + // NB: compare against the 64-bit max. A bare `-1u` is a 32-bit unsigned + // int, so `maxDBSize != -1u` would wrongly treat UINT64_MAX (the "keep + // engine default" sentinel the Rust binding sends for None) as an + // explicit value. + if (maxDBSize != std::numeric_limits::max()) { systemConfig.maxDBSize = maxDBSize; } if (autoCheckpoint >= 0) {