Remove `set_check_if_env_exists` since it's unreliable with LMDB

Signed-off-by: Victor Porof <victor.porof@gmail.com>
without.crypto
Victor Porof 4 years ago
parent bada6fec6f
commit a634920d46
  1. 10
      src/backend/impl_lmdb/environment.rs
  2. 3
      src/backend/impl_lmdb/error.rs
  3. 10
      src/backend/impl_safe/environment.rs
  4. 3
      src/backend/impl_safe/error.rs
  5. 2
      src/backend/traits.rs
  6. 3
      src/error.rs
  7. 8
      src/migrator.rs
  8. 21
      tests/env-migration.rs

@ -43,7 +43,6 @@ pub struct EnvironmentBuilderImpl {
builder: lmdb::EnvironmentBuilder,
envtype: EnvironmentType,
make_dir: bool,
check_env_exists: bool,
}
impl<'b> BackendEnvironmentBuilder<'b> for EnvironmentBuilderImpl {
@ -56,7 +55,6 @@ impl<'b> BackendEnvironmentBuilder<'b> for EnvironmentBuilderImpl {
builder: lmdb::Environment::new(),
envtype: EnvironmentType::SingleDatabase,
make_dir: false,
check_env_exists: false,
}
}
@ -91,15 +89,7 @@ impl<'b> BackendEnvironmentBuilder<'b> for EnvironmentBuilderImpl {
self
}
fn set_check_if_env_exists(&mut self, check_env_exists: bool) -> &mut Self {
self.check_env_exists = check_env_exists;
self
}
fn open(&self, path: &Path) -> Result<Self::Environment, Self::Error> {
if self.check_env_exists && !path.join("data.mdb").exists() {
return Err(ErrorImpl::EnvironmentDoesNotExistError(path.into()));
}
if !path.is_dir() {
if !self.make_dir {
return Err(ErrorImpl::DirectoryDoesNotExistError(path.into()));

@ -23,7 +23,6 @@ use crate::{
pub enum ErrorImpl {
LmdbError(lmdb::Error),
DirectoryDoesNotExistError(PathBuf),
EnvironmentDoesNotExistError(PathBuf),
IoError(io::Error),
}
@ -34,7 +33,6 @@ impl fmt::Display for ErrorImpl {
match self {
ErrorImpl::LmdbError(e) => e.fmt(fmt),
ErrorImpl::DirectoryDoesNotExistError(_) => write!(fmt, "DirectoryDoesNotExistError"),
ErrorImpl::EnvironmentDoesNotExistError(_) => write!(fmt, "EnvironmentDoesNotExistError"),
ErrorImpl::IoError(e) => e.fmt(fmt),
}
}
@ -52,7 +50,6 @@ impl Into<StoreError> for ErrorImpl {
ErrorImpl::LmdbError(lmdb::Error::ReadersFull) => StoreError::ReadersFull,
ErrorImpl::LmdbError(error) => StoreError::LmdbError(error),
ErrorImpl::DirectoryDoesNotExistError(path) => StoreError::DirectoryDoesNotExistError(path),
ErrorImpl::EnvironmentDoesNotExistError(path) => StoreError::EnvironmentDoesNotExistError(path),
ErrorImpl::IoError(error) => StoreError::IoError(error),
}
}

@ -55,7 +55,6 @@ pub struct EnvironmentBuilderImpl {
max_dbs: Option<usize>,
map_size: Option<usize>,
make_dir: bool,
check_env_exists: bool,
}
impl<'b> BackendEnvironmentBuilder<'b> for EnvironmentBuilderImpl {
@ -70,7 +69,6 @@ impl<'b> BackendEnvironmentBuilder<'b> for EnvironmentBuilderImpl {
max_dbs: None,
map_size: None,
make_dir: false,
check_env_exists: false,
}
}
@ -102,15 +100,7 @@ impl<'b> BackendEnvironmentBuilder<'b> for EnvironmentBuilderImpl {
self
}
fn set_check_if_env_exists(&mut self, check_env_exists: bool) -> &mut Self {
self.check_env_exists = check_env_exists;
self
}
fn open(&self, path: &Path) -> Result<Self::Environment, Self::Error> {
if self.check_env_exists && !path.join(DEFAULT_DB_FILENAME).exists() {
return Err(ErrorImpl::EnvironmentDoesNotExistError(path.into()));
}
if !path.is_dir() {
if !self.make_dir {
return Err(ErrorImpl::DirectoryDoesNotExistError(path.into()));

@ -30,7 +30,6 @@ pub enum ErrorImpl {
DbNotFoundError,
DbIsForeignError,
DirectoryDoesNotExistError(PathBuf),
EnvironmentDoesNotExistError(PathBuf),
IoError(io::Error),
BincodeError(BincodeError),
}
@ -47,7 +46,6 @@ impl fmt::Display for ErrorImpl {
ErrorImpl::DbNotFoundError => write!(fmt, "DbNotFoundError (safe mode)"),
ErrorImpl::DbIsForeignError => write!(fmt, "DbIsForeignError (safe mode)"),
ErrorImpl::DirectoryDoesNotExistError(_) => write!(fmt, "DirectoryDoesNotExistError (safe mode)"),
ErrorImpl::EnvironmentDoesNotExistError(_) => write!(fmt, "EnvironmentDoesNotExistError (safe mode)"),
ErrorImpl::IoError(e) => e.fmt(fmt),
ErrorImpl::BincodeError(e) => e.fmt(fmt),
}
@ -65,7 +63,6 @@ impl Into<StoreError> for ErrorImpl {
ErrorImpl::BincodeError(_) => StoreError::FileInvalid,
ErrorImpl::DbsFull => StoreError::DbsFull,
ErrorImpl::DirectoryDoesNotExistError(path) => StoreError::DirectoryDoesNotExistError(path),
ErrorImpl::EnvironmentDoesNotExistError(path) => StoreError::EnvironmentDoesNotExistError(path),
ErrorImpl::IoError(error) => StoreError::IoError(error),
_ => StoreError::SafeModeError(self),
}

@ -93,8 +93,6 @@ pub trait BackendEnvironmentBuilder<'b>: Debug + Eq + PartialEq + Copy + Clone {
fn set_make_dir_if_needed(&mut self, make_dir: bool) -> &mut Self;
fn set_check_if_env_exists(&mut self, check_env: bool) -> &mut Self;
fn open(&self, path: &Path) -> Result<Self::Environment, Self::Error>;
}

@ -87,9 +87,6 @@ pub enum StoreError {
#[fail(display = "directory does not exist or not a directory: {:?}", _0)]
DirectoryDoesNotExistError(PathBuf),
#[fail(display = "environment does not exist in directory: {:?}", _0)]
EnvironmentDoesNotExistError(PathBuf),
#[fail(display = "data error: {:?}", _0)]
DataError(DataError),

@ -96,16 +96,12 @@ macro_rules! fn_migrator {
D: std::ops::Deref<Target = Rkv<$dst_env>>,
{
use crate::backend::*;
let mut manager = crate::Manager::<$src_env>::singleton().write()?;
let mut builder = Rkv::<$src_env>::environment_builder::<$builder>();
builder.set_max_dbs(crate::env::DEFAULT_MAX_DBS);
builder.set_check_if_env_exists(true);
builder = build(builder);
let src_env = match manager.get_or_create_from_builder(path, builder, Rkv::from_builder::<$builder>) {
Err(crate::StoreError::EnvironmentDoesNotExistError(_)) => return Ok(()),
result => result,
}?;
let src_env = manager.get_or_create_from_builder(path, builder, Rkv::from_builder::<$builder>)?;
match Migrator::$migrate(src_env.read()?, dst_env) {
Err(crate::MigrateError::SourceEmpty) => return Ok(()),

@ -14,7 +14,6 @@ use tempfile::Builder;
use rkv::{
backend::{
BackendEnvironmentBuilder,
Lmdb,
SafeMode,
},
@ -181,16 +180,6 @@ fn test_migrator_round_trip() {
}
}
#[test]
#[should_panic(expected = "new succeeded: EnvironmentDoesNotExistError")]
fn test_migrator_lmdb_to_safe_0() {
let mut builder = Lmdb::new();
builder.set_check_if_env_exists(true);
let root = Builder::new().prefix("test_migrate_lmdb_to_safe").tempdir().expect("tempdir");
let _ = Rkv::from_builder::<Lmdb>(root.path(), builder).expect("new succeeded");
}
#[test]
#[should_panic(expected = "migrated: SourceEmpty")]
fn test_migrator_lmdb_to_safe_1() {
@ -232,16 +221,6 @@ fn test_migrator_lmdb_to_safe_3() {
assert_eq!(store.get(&reader, "baz").expect("read"), Some(Value::Str("héllo, yöu")));
}
#[test]
#[should_panic(expected = "new succeeded: EnvironmentDoesNotExistError")]
fn test_migrator_safe_to_lmdb_0() {
let mut builder = SafeMode::new();
builder.set_check_if_env_exists(true);
let root = Builder::new().prefix("test_migrate_safe_to_lmdb").tempdir().expect("tempdir");
let _ = Rkv::from_builder::<SafeMode>(root.path(), builder).expect("new succeeded");
}
#[test]
#[should_panic(expected = "migrated: SourceEmpty")]
fn test_migrator_safe_to_lmdb_1() {

Loading…
Cancel
Save