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

Remove internal database type checking #243

Merged
merged 2 commits into from
Feb 16, 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
9 changes: 0 additions & 9 deletions heed/examples/all-types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,6 @@ fn main() -> Result<(), Box<dyn Error>> {
let mut wtxn = env.write_txn()?;
let _db: Database<Str, Unit> = env.create_database(&mut wtxn, Some("ignored-data"))?;

// and here we try to open it with other types
// asserting that it correctly returns an error
//
// NOTE that those types are not saved upon runs and
// therefore types cannot be checked upon different runs,
// the first database opening fix the types for this run.
let result = env.create_database::<Str, SerdeJson<i32>>(&mut wtxn, Some("ignored-data"));
assert!(result.is_err());

// you can iterate over keys in order
type BEI64 = I64<BE>;

Expand Down
7 changes: 2 additions & 5 deletions heed/src/database.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::any::TypeId;
use std::borrow::Cow;
use std::ops::{Bound, RangeBounds};
use std::{any, fmt, marker, mem, ptr};
Expand Down Expand Up @@ -139,8 +138,7 @@ impl<'e, 'n, KC, DC, C> DatabaseOpenOptions<'e, 'n, KC, DC, C> {
{
assert_eq_env_txn!(self.env, rtxn);

let types = (TypeId::of::<KC>(), TypeId::of::<DC>(), TypeId::of::<C>());
match self.env.raw_init_database::<C>(rtxn.txn, self.name, types, self.flags) {
match self.env.raw_init_database::<C>(rtxn.txn, self.name, self.flags) {
Ok(dbi) => Ok(Some(Database::new(self.env.env_mut_ptr() as _, dbi))),
Err(Error::Mdb(e)) if e.not_found() => Ok(None),
Err(e) => Err(e),
Expand All @@ -164,9 +162,8 @@ impl<'e, 'n, KC, DC, C> DatabaseOpenOptions<'e, 'n, KC, DC, C> {
{
assert_eq_env_txn!(self.env, wtxn);

let types = (TypeId::of::<KC>(), TypeId::of::<DC>(), TypeId::of::<C>());
let flags = self.flags | AllDatabaseFlags::CREATE;
match self.env.raw_init_database::<C>(wtxn.txn.txn, self.name, types, flags) {
match self.env.raw_init_database::<C>(wtxn.txn.txn, self.name, flags) {
Ok(dbi) => Ok(Database::new(self.env.env_mut_ptr() as _, dbi)),
Err(e) => Err(e),
}
Expand Down
36 changes: 9 additions & 27 deletions heed/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::{
ffi::OsStr,
os::windows::io::{AsRawHandle, BorrowedHandle, RawHandle},
};
use std::{fmt, io, mem, ptr, sync};
use std::{fmt, io, mem, ptr};

use heed_traits::{Comparator, LexicographicComparator};
use once_cell::sync::Lazy;
Expand Down Expand Up @@ -266,11 +266,7 @@ impl EnvOpenOptions {
match result {
Ok(()) => {
let signal_event = Arc::new(SignalEvent::manual(false));
let inner = EnvInner {
env,
dbi_open_mutex: sync::Mutex::default(),
path: path.clone(),
};
let inner = EnvInner { env, path: path.clone() };
let env = Env(Arc::new(inner));
let cache_entry = EnvEntry {
env: Some(env.clone()),
Expand Down Expand Up @@ -303,14 +299,13 @@ pub struct Env(Arc<EnvInner>);

impl fmt::Debug for Env {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let EnvInner { env: _, dbi_open_mutex: _, path } = self.0.as_ref();
let EnvInner { env: _, path } = self.0.as_ref();
f.debug_struct("Env").field("path", &path.display()).finish_non_exhaustive()
}
}

struct EnvInner {
env: *mut ffi::MDB_env,
dbi_open_mutex: sync::Mutex<HashMap<u32, (TypeId, TypeId, TypeId)>>,
path: PathBuf,
}

Expand Down Expand Up @@ -476,6 +471,10 @@ impl Env {
}

/// Returns the size used by all the databases in the environment without the free pages.
///
/// It is crucial to configure [`EnvOpenOptions::max_dbs`] with a sufficiently large value
/// before invoking this function. All databases within the environment will be opened
/// and remain so.
pub fn non_free_pages_size(&self) -> Result<u64> {
let compute_size = |stat: ffi::MDB_stat| {
(stat.ms_leaf_pages + stat.ms_branch_pages + stat.ms_overflow_pages) as u64
Expand All @@ -490,12 +489,9 @@ impl Env {
size += compute_size(stat);

let rtxn = self.read_txn()?;
// Open the main database
let dbi = self.raw_open_dbi::<DefaultComparator>(rtxn.txn, None, 0)?;

// We don't want anyone to open an environment while we're computing the stats
// thus we take a lock on the dbi
let dbi_open = self.0.dbi_open_mutex.lock().unwrap();

// We're going to iterate on the unnamed database
let mut cursor = RoCursor::new(&rtxn, dbi)?;

Expand All @@ -512,11 +508,6 @@ impl Env {
unsafe { mdb_result(ffi::mdb_stat(rtxn.txn, dbi, stat.as_mut_ptr()))? };
let stat = unsafe { stat.assume_init() };
size += compute_size(stat);

// If the db wasn't already opened
if !dbi_open.contains_key(&dbi) {
unsafe { ffi::mdb_dbi_close(self.env_mut_ptr(), dbi) }
}
}
}

Expand Down Expand Up @@ -591,19 +582,10 @@ impl Env {
&self,
raw_txn: *mut ffi::MDB_txn,
name: Option<&str>,
types: (TypeId, TypeId, TypeId),
flags: AllDatabaseFlags,
) -> Result<u32> {
let mut lock = self.0.dbi_open_mutex.lock().unwrap();
match self.raw_open_dbi::<C>(raw_txn, name, flags.bits()) {
Ok(dbi) => {
let old_types = lock.entry(dbi).or_insert(types);
if *old_types == types {
Ok(dbi)
} else {
Err(Error::InvalidDatabaseTyping)
}
}
Ok(dbi) => Ok(dbi),
Err(e) => Err(e.into()),
}
}
Expand Down
5 changes: 0 additions & 5 deletions heed/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ pub enum Error {
Encoding(BoxedError),
/// Decoding error.
Decoding(BoxedError),
/// Incoherent types when opening a database.
InvalidDatabaseTyping,
/// Database closing in progress.
DatabaseClosing,
/// Attempt to open [`Env`] with different options.
Expand All @@ -164,9 +162,6 @@ impl fmt::Display for Error {
Error::Mdb(error) => write!(f, "{}", error),
Error::Encoding(error) => write!(f, "error while encoding: {}", error),
Error::Decoding(error) => write!(f, "error while decoding: {}", error),
Error::InvalidDatabaseTyping => {
f.write_str("database was previously opened with different types")
}
Error::DatabaseClosing => {
f.write_str("database is in a closing phase, you can't open it at the same time")
}
Expand Down
2 changes: 1 addition & 1 deletion heed/src/mdb/lmdb_ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::ptr;

pub use ffi::{
mdb_cursor_close, mdb_cursor_del, mdb_cursor_get, mdb_cursor_open, mdb_cursor_put,
mdb_dbi_close, mdb_dbi_open, mdb_del, mdb_drop, mdb_env_close, mdb_env_copyfd2, mdb_env_create,
mdb_dbi_open, mdb_del, mdb_drop, mdb_env_close, mdb_env_copyfd2, mdb_env_create,
mdb_env_get_fd, mdb_env_get_flags, mdb_env_info, mdb_env_open, mdb_env_set_mapsize,
mdb_env_set_maxdbs, mdb_env_set_maxreaders, mdb_env_stat, mdb_env_sync, mdb_filehandle_t,
mdb_get, mdb_put, mdb_reader_check, mdb_set_compare, mdb_stat, mdb_txn_abort, mdb_txn_begin,
Expand Down
Loading