Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update toolchain to 1.78 and fix 'multi_get' #42

Merged
merged 3 commits into from
May 29, 2024
Merged
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
6 changes: 3 additions & 3 deletions roxide/src/db_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ impl DbOptions {
partial_merge_fn: Option<Arc<dyn merge::MergeFn>>,
) -> Result<()> {
let cf_name = cf_name.to_string();
if self.column_families.get(&cf_name).is_none() {
if !self.column_families.contains_key(&cf_name) {
Err(Error::other_error(format!(
"Column family '{}' not found",
cf_name
Expand Down Expand Up @@ -1582,9 +1582,9 @@ pub trait OptionsExt {
/// that impl to a closure for additional customization.
///
/// If there is no logger configured, `None` is passed to the closure.
fn with_logger<R, F: FnOnce(Option<&dyn logging::RocksDbLogger>) -> R>(&mut self, func: F) -> R
fn with_logger<R, F>(&mut self, func: F) -> R
where
F: std::panic::UnwindSafe,
F: FnOnce(Option<&dyn logging::RocksDbLogger>) -> R + std::panic::UnwindSafe,
{
let options_ptr = self.inner_logger();
let raw_logger_ptr = unsafe {
Expand Down
10 changes: 2 additions & 8 deletions roxide/src/ffi_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,6 @@ pub(crate) trait DynTraitWrapper<T: ?Sized + 'static>: Sized {
/// Returns a reference to the internal `DynTraitArc` object
fn inner(&self) -> &DynTraitArc<T>;

fn inner_mut(&mut self) -> &mut DynTraitArc<T>;

/// Consume the struct and return a C pointer for passing the wrapped trait object into C code
fn into_raw_void(self) -> *mut std::ffi::c_void {
self.unwrap().into_raw_void()
Expand All @@ -374,9 +372,9 @@ pub(crate) trait DynTraitWrapper<T: ?Sized + 'static>: Sized {

/// Temporarily convert a raw void pointer to a mutable reference to the wrapper, pass it to a
/// closure, then immediately put it back to being a raw pointer again.
unsafe fn temp_from_raw_void<R, F: FnOnce(&Self) -> R>(raw: *mut std::ffi::c_void, func: F) -> R
unsafe fn temp_from_raw_void<R, F>(raw: *mut std::ffi::c_void, func: F) -> R
where
F: std::panic::UnwindSafe,
F: FnOnce(&Self) -> R + std::panic::UnwindSafe,
Self: std::panic::RefUnwindSafe,
{
let me = Self::from_raw_void(raw);
Expand Down Expand Up @@ -486,10 +484,6 @@ macro_rules! make_dyn_trait_wrapper {
fn inner(&self) -> &$crate::ffi_util::DynTraitArc<dyn $trait_type + 'static> {
&self.0
}

fn inner_mut(&mut self) -> &mut $crate::ffi_util::DynTraitArc<dyn $trait_type + 'static> {
&mut self.0
}
}

/// The wrapper type is unwind safe, in that it is written so that it cannot expose broken
Expand Down
1 change: 1 addition & 0 deletions roxide/src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub unsafe trait RocksClassSend: RocksClass {}
/// threads. If this is implemented, then the corresponding `Handle` and `ArcHandle` types will
/// also be `Sync`
#[allow(clippy::missing_safety_doc)]
#[allow(dead_code)]
pub unsafe trait RocksClassSync: RocksClass {}

/// A `Handle` is a type that wraps a pointer to one of the RocksDB FFI structs, like `rocksdb_t*`
Expand Down
15 changes: 5 additions & 10 deletions roxide/src/logging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,9 @@ private:
/// passing it to a closure.
///
/// If this database doesn't have a configured logger, passes `None` to the closure.
pub(crate) unsafe fn temp_logger_from_raw_db_ptr<
R,
F: FnOnce(Option<&(dyn RocksDbLogger + 'static)>) -> R,
>(
db_ptr: *mut std::ffi::c_void,
func: F,
) -> R
pub(crate) unsafe fn temp_logger_from_raw_db_ptr<R, F>(db_ptr: *mut std::ffi::c_void, func: F) -> R
where
F: std::panic::UnwindSafe,
F: FnOnce(Option<&(dyn RocksDbLogger + 'static)>) -> R + std::panic::UnwindSafe,
{
let cpp_logger_ptr = cpp!([db_ptr as "rocksdb::DB*"] -> *mut std::ffi::c_void as "void*" {
std::string id;
Expand Down Expand Up @@ -172,6 +166,7 @@ static JSON_EVENT_PREFIX: &str = "EVENT_LOG_v1 ";
pub trait LoggingContext: LabelSet + AsSpan {
/// Gets a copy of the fields in this context, in a `HashMap`. This is mostly intended for
/// testing.
#[allow(dead_code)]
fn fields(&self) -> HashMap<String, String> {
// LabelSet already exposes all labels as a hash map. Just need to convert the keys and
// values into owned strings
Expand Down Expand Up @@ -524,12 +519,12 @@ impl CppLoggerWrapper {
/// this pointer isn't valid you'll get nasty UBF), and passes that boxed logger to a closure.
///
/// This closure can call methods on that logger but it cannot take ownership.
pub(crate) unsafe fn with_raw_boxed_logger<R, F: FnOnce(&(dyn RocksDbLogger + 'static)) -> R>(
pub(crate) unsafe fn with_raw_boxed_logger<R, F>(
logger_ptr: *mut std::ffi::c_void,
func: F,
) -> R
where
F: std::panic::UnwindSafe,
F: FnOnce(&(dyn RocksDbLogger + 'static)) -> R + std::panic::UnwindSafe,
{
Self::temp_from_raw_void(logger_ptr, move |wrapper| {
wrapper.with_inner(|inner: &Arc<_>| {
Expand Down
8 changes: 4 additions & 4 deletions roxide/src/ops/begin_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,7 @@ mod test {
/// in the presence of conflicts.
///
/// This is generic over the actual database type, as long as it supports transactions.
fn conflict_impl(db: impl DbLike + BeginTrans) -> Result<()> {
fn conflict_impl(db: impl BeginTrans) -> Result<()> {
let cf = db.get_cf("default").unwrap();
let keys = generate_test_keys();

Expand Down Expand Up @@ -1012,12 +1012,12 @@ mod test {

/// Async version of `conflig_impl`
#[allow(clippy::async_yields_async)] // The `async` block yielding a future is needed here to satisfy the borrow checker
async fn conflict_impl_async<DBT: DbLike + BeginTrans>(db: DBT) -> Result<()>
async fn conflict_impl_async<DBT>(db: DBT) -> Result<()>
where
<DBT as RocksOpBase>::HandleType: Sync + Send,
DBT::TransactionOptionsType: Send + 'static,
DBT::TransactionOptionsNativeType: 'static,
DBT: Sync + Clone + 'static,
DBT: DbLike + BeginTrans + Sync + Clone + 'static,
{
let cf = db.get_cf("default").unwrap();
let keys = generate_test_keys();
Expand Down Expand Up @@ -1163,7 +1163,7 @@ mod test {
///
/// This is generic over the actual database type, as long as it supports transactions.
#[allow(clippy::float_cmp)] // Testing that the sum is != 0 is the correct assertion here
fn no_conflicts_impl(db: impl DbLike + BeginTrans) -> Result<()> {
fn no_conflicts_impl(db: impl BeginTrans) -> Result<()> {
let cf = db.get_cf("default").unwrap();

let mut all_keys =
Expand Down
13 changes: 13 additions & 0 deletions roxide/src/ops/get.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ unsafe fn raw_multi_get_impl<K: BinaryStr>(
// Keed all of the keys in a contiguous vector to use MultiGet
let keys = keys.into_iter().collect::<Vec<_>>();
let num_keys = keys.len();
if num_keys == 0 {
return Ok(Vec::new());
}

let cf_ptr = cf.as_ptr();
let options_ptr = options.as_ptr();

Expand Down Expand Up @@ -1201,4 +1205,13 @@ mod test {

Ok(())
}

#[test]
fn multi_get_empty() -> Result<()> {
let path = TempDbPath::new();
let db = Db::open(&path, None)?;
let cf = db.get_cf("default").unwrap();
let _ = db.multi_get(&cf, std::iter::empty::<[u8; 0]>(), None)?;
Ok(())
}
}
2 changes: 2 additions & 0 deletions roxide/src/ops/get_labels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ use crate::db::ColumnFamilyLike;
use crate::db::DbLike;
use crate::labels;

#[allow(dead_code)]
pub(crate) trait GetDbLabels {
fn get_db_labels(&self) -> labels::DatabaseLabels;
fn get_db_op_labels(&self, op_name: &'static str) -> labels::DatabaseOperationLabels;
}

#[allow(dead_code)]
pub(crate) trait GetColumnFamilyLabels {
fn get_cf_labels(&self) -> labels::ColumnFamilyLabels;
fn get_cf_op_labels(&self, op_name: &'static str) -> labels::ColumnFamilyOperationLabels;
Expand Down
8 changes: 4 additions & 4 deletions roxide/src/ops/with_logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ pub(crate) trait WithLogger {
/// If the object is associated with a C++ RocksDB logger which wraps a Rust implementation in
/// the form of `RocksDbLogger`, then calls a closure with a reference to that logger, or
/// `None` if no such logger is configured.
fn with_logger<R, F: FnOnce(Option<&(dyn RocksDbLogger + 'static)>) -> R>(&self, func: F) -> R
fn with_logger<R, F>(&self, func: F) -> R
where
F: std::panic::UnwindSafe;
F: FnOnce(Option<&(dyn RocksDbLogger + 'static)>) -> R + std::panic::UnwindSafe;
}

impl<DB: GetDbPtr> WithLogger for DB {
fn with_logger<R, F: FnOnce(Option<&(dyn RocksDbLogger + 'static)>) -> R>(&self, func: F) -> R
fn with_logger<R, F>(&self, func: F) -> R
where
F: std::panic::UnwindSafe,
F: FnOnce(Option<&(dyn RocksDbLogger + 'static)>) -> R + std::panic::UnwindSafe,
{
let db_ptr = <Self as crate::ops::GetDbPtr>::get_db_ptr(self);

Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.75.0
1.78.0
Loading