Skip to content

Commit

Permalink
Make migration methods and error handling less surprising
Browse files Browse the repository at this point in the history
Signed-off-by: Victor Porof <[email protected]>
  • Loading branch information
victorporof committed Jul 29, 2020
1 parent 6e04f6a commit d74250b
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 28 deletions.
56 changes: 40 additions & 16 deletions src/migrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,19 @@
//!
//! The destination environment should be empty of data, otherwise an error is returned.
//!
//! There are 3 versions of the migration methods:
//! * `migrate_<src>_to_<dst>`, where `<src>` and `<dst>` are the source and destination
//! environment types. You're responsive with opening both these environments, handling
//! all errors, and performing any cleanup if necessary.
//! * `open_and_migrate_<src>_to_<dst>`, which is similar the the above, but automatically
//! attempts to open the source environment and delete all of its supporting files if
//! there's no other environment open at that path. You're still responsible with
//! handling all errors.
//! * `easy_migrate_<src>_to_<dst>` which is similar to the above, but ignores the
//! migration and doesn't delete any files if the source environment is invalid
//! (corrupted), unavailable (path not found or incompatible with configuration), or
//! empty (database has no records).
//!
//! The tool currently has these limitations:
//!
//! 1. It doesn't support migration from environments created with
Expand Down Expand Up @@ -86,10 +99,10 @@ macro_rules! fn_migrator {
}
};

($migrate:tt, $name:tt, $builder:tt, $src_env:ty, $dst_env:ty) => {
/// Same as the other migration methods, but automatically attempts to open the source
/// environment, ignores it if it's invalid, if it doesn't exist or if it's empty.
/// Finally, attempts to delete all of its supporting files.
(open $migrate:tt, $name:tt, $builder:tt, $src_env:ty, $dst_env:ty) => {
/// Same as the non `open_*` migration method, but automatically attempts to open the
/// source environment. Finally, deletes all of its supporting files if there's no other
/// environment open at that path.
pub fn $name<F, D>(path: &std::path::Path, build: F, dst_env: D) -> Result<(), MigrateError>
where
F: FnOnce(crate::backend::$builder) -> crate::backend::$builder,
Expand All @@ -102,24 +115,34 @@ macro_rules! fn_migrator {
builder.set_max_dbs(crate::env::DEFAULT_MAX_DBS);
builder = build(builder);

let src_env = match manager.get_or_create_from_builder(path, builder, Rkv::from_builder::<$builder>) {
Err(crate::StoreError::FileInvalid) => return Ok(()),
Err(crate::StoreError::IoError(ref e)) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()),
Err(crate::StoreError::UnsuitableEnvironmentPath(_)) => return Ok(()),
result => result,
}?;

match Migrator::$migrate(src_env.read()?, dst_env) {
Err(crate::MigrateError::SourceEmpty) => return Ok(()),
result => result,
}?;
let src_env = manager.get_or_create_from_builder(path, builder, Rkv::from_builder::<$builder>)?;
Migrator::$migrate(src_env.read()?, dst_env)?;

drop(src_env);
manager.try_close_and_delete(path)?;

Ok(())
}
};

(easy $migrate:tt, $name:tt, $src_env:ty, $dst_env:ty) => {
/// Same as the `open_*` migration method, but ignores the migration and doesn't delete
/// any files if the source environment is invalid (corrupted), unavailable, or empty.
pub fn $name<D>(path: &std::path::Path, dst_env: D) -> Result<(), MigrateError>
where
D: std::ops::Deref<Target = Rkv<$dst_env>>,
{
match Migrator::$migrate(path, |builder| builder, dst_env) {
Err(crate::MigrateError::StoreError(crate::StoreError::FileInvalid)) => Ok(()),
Err(crate::MigrateError::StoreError(crate::StoreError::IoError(_))) => Ok(()),
Err(crate::MigrateError::StoreError(crate::StoreError::UnsuitableEnvironmentPath(_))) => Ok(()),
Err(crate::MigrateError::SourceEmpty) => Ok(()),
result => result,
}?;

Ok(())
}
};
}

macro_rules! fns_migrator {
Expand All @@ -132,7 +155,8 @@ macro_rules! fns_migrator {
($name:tt, $src:tt, $dst:tt) => {
paste::item! {
fn_migrator!($name, [<$src:camel Environment>], [<$dst:camel Environment>]);
fn_migrator!($name, [<auto_ $name>], [<$src:camel>], [<$src:camel Environment>], [<$dst:camel Environment>]);
fn_migrator!(open $name, [<open_and_ $name>], [<$src:camel>], [<$src:camel Environment>], [<$dst:camel Environment>]);
fn_migrator!(easy [<open_and_ $name>], [<easy_ $name>], [<$src:camel Environment>], [<$dst:camel Environment>]);
}
};
}
Expand Down
28 changes: 16 additions & 12 deletions tests/env-migration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ fn test_simple_migrator_lmdb_to_safe() {
assert_eq!(store.get(&reader, "bar").expect("read"), Some(Value::Bool(true)));
assert_eq!(store.get(&reader, "baz").expect("read"), Some(Value::Str("héllo, yöu")));
}
// Easy migrate.
// Open and migrate.
{
let dst_env = Rkv::new::<SafeMode>(root.path()).expect("new succeeded");
Migrator::auto_migrate_lmdb_to_safe_mode(root.path(), |builder| builder, &dst_env).expect("migrated");
Migrator::open_and_migrate_lmdb_to_safe_mode(root.path(), |builder| builder, &dst_env).expect("migrated");
}
// Verify that the database was indeed migrated.
{
Expand Down Expand Up @@ -117,10 +117,10 @@ fn test_simple_migrator_safe_to_lmdb() {
assert_eq!(store.get(&reader, "bar").expect("read"), Some(Value::Bool(true)));
assert_eq!(store.get(&reader, "baz").expect("read"), Some(Value::Str("héllo, yöu")));
}
// Easy migrate.
// Open and migrate.
{
let dst_env = Rkv::new::<Lmdb>(root.path()).expect("new succeeded");
Migrator::auto_migrate_safe_mode_to_lmdb(root.path(), |builder| builder, &dst_env).expect("migrated");
Migrator::open_and_migrate_safe_mode_to_lmdb(root.path(), |builder| builder, &dst_env).expect("migrated");
}
// Verify that the database was indeed migrated.
{
Expand Down Expand Up @@ -150,15 +150,15 @@ fn test_migrator_round_trip() {
populate_store!(&src_env);
src_env.sync(true).expect("synced");
}
// Easy migrate.
// Open and migrate.
{
let dst_env = Rkv::new::<SafeMode>(root.path()).expect("new succeeded");
Migrator::auto_migrate_lmdb_to_safe_mode(root.path(), |builder| builder, &dst_env).expect("migrated");
Migrator::open_and_migrate_lmdb_to_safe_mode(root.path(), |builder| builder, &dst_env).expect("migrated");
}
// Easy migrate back.
// Open and migrate back.
{
let dst_env = Rkv::new::<Lmdb>(root.path()).expect("new succeeded");
Migrator::auto_migrate_safe_mode_to_lmdb(root.path(), |builder| builder, &dst_env).expect("migrated");
Migrator::open_and_migrate_safe_mode_to_lmdb(root.path(), |builder| builder, &dst_env).expect("migrated");
}
// Verify that the database was indeed migrated twice.
{
Expand Down Expand Up @@ -188,8 +188,10 @@ fn test_migrator_no_dir_1() {
let root = Builder::new().prefix("test_migrator_no_dir").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

// This won't fail with IoError even though the path is a bogus path, because this
// is the "easy mode" migration which automatically handles (ignores) this error.
let dst_env = Rkv::new::<SafeMode>(root.path()).expect("new succeeded");
Migrator::auto_migrate_lmdb_to_safe_mode(Path::new("bogus"), |builder| builder, &dst_env).expect("migrated");
Migrator::easy_migrate_lmdb_to_safe_mode(Path::new("bogus"), &dst_env).expect("migrated");

let mut datamdb = root.path().to_path_buf();
let mut lockmdb = root.path().to_path_buf();
Expand All @@ -207,8 +209,10 @@ fn test_migrator_no_dir_2() {
let root = Builder::new().prefix("test_migrator_no_dir").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

// This won't fail with IoError even though the path is a bogus path, because this
// is the "easy mode" migration which automatically handles (ignores) this error.
let dst_env = Rkv::new::<Lmdb>(root.path()).expect("new succeeded");
Migrator::auto_migrate_safe_mode_to_lmdb(Path::new("bogus"), |builder| builder, &dst_env).expect("migrated");
Migrator::easy_migrate_safe_mode_to_lmdb(Path::new("bogus"), &dst_env).expect("migrated");

let mut datamdb = root.path().to_path_buf();
let mut lockmdb = root.path().to_path_buf();
Expand All @@ -232,7 +236,7 @@ fn test_migrator_invalid_1() {
// This won't fail with FileInvalid even though the database is a bogus file, because this
// is the "easy mode" migration which automatically handles (ignores) this error.
let dst_env = Rkv::new::<SafeMode>(root.path()).expect("new succeeded");
Migrator::auto_migrate_lmdb_to_safe_mode(root.path(), |builder| builder, &dst_env).expect("migrated");
Migrator::easy_migrate_lmdb_to_safe_mode(root.path(), &dst_env).expect("migrated");

let mut datamdb = root.path().to_path_buf();
let mut lockmdb = root.path().to_path_buf();
Expand All @@ -256,7 +260,7 @@ fn test_migrator_invalid_2() {
// This won't fail with FileInvalid even though the database is a bogus file, because this
// is the "easy mode" migration which automatically handles (ignores) this error.
let dst_env = Rkv::new::<Lmdb>(root.path()).expect("new succeeded");
Migrator::auto_migrate_safe_mode_to_lmdb(root.path(), |builder| builder, &dst_env).expect("migrated");
Migrator::easy_migrate_safe_mode_to_lmdb(root.path(), &dst_env).expect("migrated");

let mut datamdb = root.path().to_path_buf();
let mut lockmdb = root.path().to_path_buf();
Expand Down

0 comments on commit d74250b

Please sign in to comment.