Skip to content

Commit

Permalink
Make easy migrator more reslient against situations where old files d…
Browse files Browse the repository at this point in the history 
…on't get deleted

Signed-off-by: Victor Porof <[email protected]>
  • Loading branch information
@victorporof
victorporof committed Oct 9, 2020
1 parent 2b0ecec commit 2d93f06
Show file tree
Hide file tree
Showing 2 changed files with 140 additions and 21 deletions.
20 changes: 13 additions & 7 deletions src/migrator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
//! 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).
//! (corrupted), unavailable (path not accessible or incompatible with configuration),
//! or empty (database has no records).
//!
//! The tool currently has these limitations:
//!
Expand Down Expand Up @@ -100,9 +100,9 @@ macro_rules! fn_migrator {
};

(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.
/// Same as the the `migrate_x_to_y` migration method above, 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 and the migration succeeded.
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 @@ -126,8 +126,13 @@ macro_rules! fn_migrator {
};

(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.
/// Same as the `open_and_migrate_x_to_y` migration method above, but ignores the
/// migration and doesn't delete any files if the following conditions apply:
/// - Source environment is invalid (corrupted), unavailable, or empty.
/// - Destination environment is not empty.
/// Use this instead of the other migration methods if:
/// - You're not concerned by throwing away old data and starting fresh with a new store.
/// - You'll never want to overwrite data in the new store from the old store.
pub fn $name<D>(path: &std::path::Path, dst_env: D) -> Result<(), MigrateError>
where
D: std::ops::Deref<Target = Rkv<$dst_env>>,
Expand All @@ -137,6 +142,7 @@ macro_rules! fn_migrator {
Err(crate::MigrateError::StoreError(crate::StoreError::IoError(_))) => Ok(()),
Err(crate::MigrateError::StoreError(crate::StoreError::UnsuitableEnvironmentPath(_))) => Ok(()),
Err(crate::MigrateError::SourceEmpty) => Ok(()),
Err(crate::MigrateError::DestinationNotEmpty) => Ok(()),
result => result,
}?;

Expand Down
141 changes: 127 additions & 14 deletions tests/env-migration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ use tempfile::Builder;
use rkv::{
backend::{
Lmdb,
LmdbEnvironment,
SafeMode,
SafeModeEnvironment,
},
Manager,
Migrator,
Rkv,
StoreOptions,
Expand All @@ -38,8 +41,8 @@ macro_rules! populate_store {
}

#[test]
fn test_simple_migrator_lmdb_to_safe() {
let root = Builder::new().prefix("test_simple_migrator_lmdb_to_safe").tempdir().expect("tempdir");
fn test_open_migrator_lmdb_to_safe() {
let root = Builder::new().prefix("test_open_migrator_lmdb_to_safe").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

// Populate source environment and persist to disk.
Expand Down Expand Up @@ -92,8 +95,8 @@ fn test_simple_migrator_lmdb_to_safe() {
}

#[test]
fn test_simple_migrator_safe_to_lmdb() {
let root = Builder::new().prefix("test_simple_migrator_safe_to_lmdb").tempdir().expect("tempdir");
fn test_open_migrator_safe_to_lmdb() {
let root = Builder::new().prefix("test_open_migrator_safe_to_lmdb").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

// Populate source environment and persist to disk.
Expand Down Expand Up @@ -140,8 +143,8 @@ fn test_simple_migrator_safe_to_lmdb() {
}

#[test]
fn test_migrator_round_trip() {
let root = Builder::new().prefix("test_simple_migrator_lmdb_to_safe").tempdir().expect("tempdir");
fn test_open_migrator_round_trip() {
let root = Builder::new().prefix("test_open_migrator_lmdb_to_safe").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

// Populate source environment and persist to disk.
Expand Down Expand Up @@ -184,8 +187,8 @@ fn test_migrator_round_trip() {
}

#[test]
fn test_migrator_no_dir_1() {
let root = Builder::new().prefix("test_migrator_no_dir").tempdir().expect("tempdir");
fn test_easy_migrator_no_dir_1() {
let root = Builder::new().prefix("test_easy_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
Expand All @@ -205,8 +208,8 @@ fn test_migrator_no_dir_1() {
}

#[test]
fn test_migrator_no_dir_2() {
let root = Builder::new().prefix("test_migrator_no_dir").tempdir().expect("tempdir");
fn test_easy_migrator_no_dir_2() {
let root = Builder::new().prefix("test_easy_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
Expand All @@ -226,8 +229,8 @@ fn test_migrator_no_dir_2() {
}

#[test]
fn test_migrator_invalid_1() {
let root = Builder::new().prefix("test_migrator_invalid").tempdir().expect("tempdir");
fn test_easy_migrator_invalid_1() {
let root = Builder::new().prefix("test_easy_migrator_invalid").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

let dbfile = root.path().join("data.mdb");
Expand All @@ -250,8 +253,8 @@ fn test_migrator_invalid_1() {
}

#[test]
fn test_migrator_invalid_2() {
let root = Builder::new().prefix("test_migrator_invalid").tempdir().expect("tempdir");
fn test_easy_migrator_invalid_2() {
let root = Builder::new().prefix("test_easy_migrator_invalid").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

let dbfile = root.path().join("data.safe.bin");
Expand Down Expand Up @@ -354,3 +357,113 @@ fn test_migrator_safe_to_lmdb_3() {
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")));
}

#[test]
fn test_easy_migrator_failed_migration_1() {
let root = Builder::new().prefix("test_easy_migrator_failed_migration_1").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

let dbfile = root.path().join("data.mdb");
fs::write(&dbfile, "bogus").expect("bogus dbfile created");

// 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::easy_migrate_lmdb_to_safe_mode(root.path(), &dst_env).expect("migrated");

// Populate destination environment and persist to disk.
populate_store!(&dst_env);
dst_env.sync(true).expect("synced");

// Delete bogus file and create a valid source environment in its place.
fs::remove_file(&dbfile).expect("bogus dbfile removed");
let src_env = Rkv::new::<Lmdb>(root.path()).expect("new succeeded");
populate_store!(&src_env);
src_env.sync(true).expect("synced");

// Attempt to migrate again. This should *NOT* fail with DestinationNotEmpty.
Migrator::easy_migrate_lmdb_to_safe_mode(root.path(), &dst_env).expect("migrated");
}

#[test]
fn test_easy_migrator_failed_migration_2() {
let root = Builder::new().prefix("test_easy_migrator_failed_migration_2").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

let dbfile = root.path().join("data.safe.bin");
fs::write(&dbfile, "bogus").expect("bogus dbfile created");

// 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::easy_migrate_safe_mode_to_lmdb(root.path(), &dst_env).expect("migrated");

// Populate destination environment and persist to disk.
populate_store!(&dst_env);
dst_env.sync(true).expect("synced");

// Delete bogus file and create a valid source environment in its place.
fs::remove_file(&dbfile).expect("bogus dbfile removed");
let src_env = Rkv::new::<SafeMode>(root.path()).expect("new succeeded");
populate_store!(&src_env);
src_env.sync(true).expect("synced");

// Attempt to migrate again. This should *NOT* fail with DestinationNotEmpty.
Migrator::easy_migrate_safe_mode_to_lmdb(root.path(), &dst_env).expect("migrated");
}

fn test_easy_migrator_from_manager_failed_migration_1() {
let root = Builder::new().prefix("test_easy_migrator_from_manager_failed_migration_1").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

{
let mut src_manager = Manager::<LmdbEnvironment>::singleton().write().unwrap();
let created_src_arc = src_manager.get_or_create(root.path(), Rkv::new::<Lmdb>).unwrap();
let src_env = created_src_arc.read().unwrap();
populate_store!(&src_env);
src_env.sync(true).expect("synced");
}
{
let mut dst_manager = Manager::<SafeModeEnvironment>::singleton().write().unwrap();
let created_dst_arc_1 = dst_manager.get_or_create(root.path(), Rkv::new::<SafeMode>).unwrap();
let dst_env_1 = created_dst_arc_1.read().unwrap();
populate_store!(&dst_env_1);
}

// Attempt to migrate again in a new env. This should *NOT* fail with DestinationNotEmpty.
let dst_manager = Manager::<SafeModeEnvironment>::singleton().read().unwrap();
let created_dst_arc_2 = dst_manager.get(root.path()).unwrap().unwrap();
let dst_env_2 = created_dst_arc_2.read().unwrap();
Migrator::easy_migrate_lmdb_to_safe_mode(root.path(), dst_env_2).expect("migrated");
}

fn test_easy_migrator_from_manager_failed_migration_2() {
let root = Builder::new().prefix("test_easy_migrator_from_manager_failed_migration_2").tempdir().expect("tempdir");
fs::create_dir_all(root.path()).expect("dir created");

{
let mut src_manager = Manager::<SafeModeEnvironment>::singleton().write().unwrap();
let created_src_arc = src_manager.get_or_create(root.path(), Rkv::new::<SafeMode>).unwrap();
let src_env = created_src_arc.read().unwrap();
populate_store!(&src_env);
src_env.sync(true).expect("synced");
}
{
let mut dst_manager = Manager::<LmdbEnvironment>::singleton().write().unwrap();
let created_dst_arc_1 = dst_manager.get_or_create(root.path(), Rkv::new::<Lmdb>).unwrap();
let dst_env_1 = created_dst_arc_1.read().unwrap();
populate_store!(&dst_env_1);
}

// Attempt to migrate again in a new env. This should *NOT* fail with DestinationNotEmpty.
let dst_manager = Manager::<LmdbEnvironment>::singleton().read().unwrap();
let created_dst_arc_2 = dst_manager.get(root.path()).unwrap().unwrap();
let dst_env_2 = created_dst_arc_2.read().unwrap();
Migrator::easy_migrate_safe_mode_to_lmdb(root.path(), dst_env_2).expect("migrated");
}

#[test]
fn test_easy_migrator_from_manager_failed_migration() {
test_easy_migrator_from_manager_failed_migration_1();
test_easy_migrator_from_manager_failed_migration_2();
}

0 comments on commit 2d93f06

Please sign in to comment.