From b7d243368b3fa79577712fae9a0fad05dbdeca3b Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Wed, 6 Jul 2016 12:05:23 +0200 Subject: [PATCH] Extend migration framework (#1546) * partially done alternate migration scheme * finish altering migration framework * migrate tests to new migration framework * address comments * remove superfluous newline [ci skip] * TempIdx -> TempIndex [ci skip] --- ethcore/src/migrations/extras/v6.rs | 4 +- parity/migration.rs | 55 +++----- util/src/migration/db_impl.rs | 55 -------- util/src/migration/manager.rs | 117 ---------------- util/src/migration/mod.rs | 205 ++++++++++++++++++++++++++-- util/src/migration/tests.rs | 107 +++++++++------ 6 files changed, 279 insertions(+), 264 deletions(-) delete mode 100644 util/src/migration/db_impl.rs delete mode 100644 util/src/migration/manager.rs diff --git a/ethcore/src/migrations/extras/v6.rs b/ethcore/src/migrations/extras/v6.rs index 9b97e594b..3b8160e67 100644 --- a/ethcore/src/migrations/extras/v6.rs +++ b/ethcore/src/migrations/extras/v6.rs @@ -1,4 +1,4 @@ -use util::migration::Migration; +use util::migration::SimpleMigration; /// This migration reduces the sizes of keys and moves `ExtrasIndex` byte from back to the front. pub struct ToV6; @@ -17,7 +17,7 @@ impl ToV6 { } } -impl Migration for ToV6 { +impl SimpleMigration for ToV6 { fn version(&self) -> u32 { 6 } diff --git a/parity/migration.rs b/parity/migration.rs index df2c2116f..6b5c7de99 100644 --- a/parity/migration.rs +++ b/parity/migration.rs @@ -19,8 +19,7 @@ use std::fs::File; use std::io::{Read, Write, Error as IoError, ErrorKind}; use std::path::PathBuf; use std::fmt::{Display, Formatter, Error as FmtError}; -use util::migration::{Manager as MigrationManager, Config as MigrationConfig, MigrationIterator}; -use util::kvdb::{Database, DatabaseConfig, CompactionProfile}; +use util::migration::{Manager as MigrationManager, Config as MigrationConfig, Error as MigrationError}; use ethcore::migrations; /// Database is assumed to be at default version, when no version file is found. @@ -65,6 +64,15 @@ impl From for Error { } } +impl From for Error { + fn from(err: MigrationError) -> Self { + match err { + MigrationError::Io(e) => Error::Io(e), + _ => Error::MigrationFailed, + } + } +} + /// Returns the version file path. fn version_file_path(path: &PathBuf) -> PathBuf { let mut file_path = path.clone(); @@ -109,14 +117,6 @@ fn extras_database_path(path: &PathBuf) -> PathBuf { extras_path } -/// Temporary database path used for migration. -fn temp_database_path(path: &PathBuf) -> PathBuf { - let mut temp_path = path.clone(); - temp_path.pop(); - temp_path.push("temp_migration"); - temp_path -} - /// Database backup fn backup_database_path(path: &PathBuf) -> PathBuf { let mut backup_path = path.clone(); @@ -146,44 +146,27 @@ fn extras_database_migrations() -> Result { } /// Migrates database at given position with given migration rules. -fn migrate_database(version: u32, path: PathBuf, migrations: MigrationManager) -> Result<(), Error> { +fn migrate_database(version: u32, db_path: PathBuf, migrations: MigrationManager) -> Result<(), Error> { // check if migration is needed if !migrations.is_needed(version) { return Ok(()) } - let temp_path = temp_database_path(&path); - let backup_path = backup_database_path(&path); - // remote the dir if it exists - let _ = fs::remove_dir_all(&temp_path); + let backup_path = backup_database_path(&db_path); + // remove the backup dir if it exists let _ = fs::remove_dir_all(&backup_path); - { - let db_config = DatabaseConfig { - prefix_size: None, - max_open_files: 64, - cache_size: None, - compaction: CompactionProfile::default(), - }; - - // open old database - let old = try!(Database::open(&db_config, path.to_str().unwrap()).map_err(|_| Error::MigrationFailed)); - - // create new database - let mut temp = try!(Database::open(&db_config, temp_path.to_str().unwrap()).map_err(|_| Error::MigrationFailed)); - - // migrate old database to the new one - try!(migrations.execute(MigrationIterator::from(old.iter()), version, &mut temp).map_err(|_| Error::MigrationFailed)); - } + // migrate old database to the new one + let temp_path = try!(migrations.execute(&db_path, version)); // create backup - try!(fs::rename(&path, &backup_path)); + try!(fs::rename(&db_path, &backup_path)); // replace the old database with the new one - if let Err(err) = fs::rename(&temp_path, &path) { + if let Err(err) = fs::rename(&temp_path, &db_path) { // if something went wrong, bring back backup - try!(fs::rename(&backup_path, path)); - return Err(From::from(err)); + try!(fs::rename(&backup_path, &db_path)); + return Err(err.into()); } // remove backup diff --git a/util/src/migration/db_impl.rs b/util/src/migration/db_impl.rs deleted file mode 100644 index 9adf22a4a..000000000 --- a/util/src/migration/db_impl.rs +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright 2015, 2016 Ethcore (UK) Ltd. -// This file is part of Parity. - -// Parity is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Parity is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Parity. If not, see . - -//! `kvdb::Database` as `migration::Destination` - -use std::collections::BTreeMap; -use kvdb::{Database, DatabaseIterator, DBTransaction}; -use migration::{Destination, Error}; - -/// Database iterator with `Item` complient with migration `Manager` interface. -pub struct MigrationIterator { - iter: DatabaseIterator, -} - -impl From for MigrationIterator { - fn from(iter: DatabaseIterator) -> Self { - MigrationIterator { - iter: iter - } - } -} - -impl Iterator for MigrationIterator { - type Item = (Vec, Vec); - - fn next(&mut self) -> Option { - self.iter.next().map(|(k, v)| (k.to_vec(), v.to_vec())) - } -} - -impl Destination for Database { - fn commit(&mut self, batch: BTreeMap, Vec>) -> Result<(), Error> { - let transaction = DBTransaction::new(); - - for keypair in &batch { - try!(transaction.put(&keypair.0, &keypair.1).map_err(Error::Custom)) - } - - self.write(transaction).map_err(Error::Custom) - } -} - diff --git a/util/src/migration/manager.rs b/util/src/migration/manager.rs deleted file mode 100644 index bd21d1637..000000000 --- a/util/src/migration/manager.rs +++ /dev/null @@ -1,117 +0,0 @@ -// Copyright 2015, 2016 Ethcore (UK) Ltd. -// This file is part of Parity. - -// Parity is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. - -// Parity is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. - -// You should have received a copy of the GNU General Public License -// along with Parity. If not, see . - -//! Migration manager - -use std::collections::BTreeMap; -use migration::{Migration, Destination}; - -/// Migration error. -#[derive(Debug)] -pub enum Error { - /// Error returned when it is impossible to add new migration rules. - CannotAddMigration, - /// Error returned when migration from specific version can not be performed. - MigrationImpossible, - /// Custom error. - Custom(String), -} - -/// Migration config. -pub struct Config { - /// Defines how many elements should be migrated at once. - pub batch_size: usize, -} - -impl Default for Config { - fn default() -> Self { - Config { - batch_size: 1024, - } - } -} - -/// Manages database migration. -pub struct Manager { - config: Config, - migrations: Vec>, -} - -impl Manager { - /// Creates new migration manager with given configuration. - pub fn new(config: Config) -> Self { - Manager { - config: config, - migrations: vec![] - } - } - - /// Adds new migration rules. - pub fn add_migration(&mut self, migration: T) -> Result<(), Error> where T: Migration { - let version_match = match self.migrations.last() { - Some(last) => last.version() + 1 == migration.version(), - None => true, - }; - - match version_match { - true => Ok(self.migrations.push(Box::new(migration))), - false => Err(Error::CannotAddMigration), - } - } - - /// Performs migration to destination. - pub fn execute(&self, db_iter: D, version: u32, destination: &mut Destination) -> Result<(), Error> where - D: Iterator, Vec)> { - - let migrations = try!(self.migrations_from(version).ok_or(Error::MigrationImpossible)); - - let mut batch: BTreeMap, Vec> = BTreeMap::new(); - - for keypair in db_iter { - let migrated = migrations.iter().fold(Some(keypair), |migrated, migration| { - migrated.and_then(|(key, value)| migration.simple_migrate(key, value)) - }); - - if let Some((key, value)) = migrated { - batch.insert(key, value); - } - - if batch.len() == self.config.batch_size { - try!(destination.commit(batch)); - batch = BTreeMap::new(); - } - } - - try!(destination.commit(batch)); - - Ok(()) - } - - /// Returns true if migration is needed. - pub fn is_needed(&self, version: u32) -> bool { - match self.migrations.last() { - Some(last) => version < last.version(), - None => false, - } - } - - fn migrations_from(&self, version: u32) -> Option<&[Box]> { - // index of the first required migration - let position = self.migrations.iter().position(|m| m.version() == version + 1); - position.map(|p| &self.migrations[p..]) - } -} - diff --git a/util/src/migration/mod.rs b/util/src/migration/mod.rs index 83d42d098..4a51893c0 100644 --- a/util/src/migration/mod.rs +++ b/util/src/migration/mod.rs @@ -15,19 +15,58 @@ // along with Parity. If not, see . //! DB Migration module. - -mod db_impl; -mod manager; - #[cfg(test)] mod tests; -pub use self::manager::{Error, Config, Manager}; -pub use self::db_impl::MigrationIterator; use std::collections::BTreeMap; +use std::fs; +use std::path::{Path, PathBuf}; -/// Single migration. +use ::kvdb::{CompactionProfile, Database, DatabaseConfig, DBTransaction}; + +/// Migration config. +pub struct Config { + /// Defines how many elements should be migrated at once. + pub batch_size: usize, +} + +impl Default for Config { + fn default() -> Self { + Config { + batch_size: 1024, + } + } +} + +/// Migration error. +#[derive(Debug)] +pub enum Error { + /// Error returned when it is impossible to add new migration rules. + CannotAddMigration, + /// Error returned when migration from specific version can not be performed. + MigrationImpossible, + /// Io Error. + Io(::std::io::Error), + /// Custom error. + Custom(String), +} + +impl From<::std::io::Error> for Error { + fn from(e: ::std::io::Error) -> Self { + Error::Io(e) + } +} + +/// A generalized migration from the given db to a destination db. pub trait Migration: 'static { + /// Version of the database after the migration. + fn version(&self) -> u32; + /// Migrate a source to a destination. + fn migrate(&self, source: &Database, config: &Config, destination: &mut Database) -> Result<(), Error>; +} + +/// A simple migration over key-value pairs. +pub trait SimpleMigration: 'static { /// Version of database after the migration. fn version(&self) -> u32; /// Should migrate existing object to new database. @@ -35,8 +74,152 @@ pub trait Migration: 'static { fn simple_migrate(&self, key: Vec, value: Vec) -> Option<(Vec, Vec)>; } -/// Migration destination. -pub trait Destination { - /// Called on destination to commit batch of migrated entries. - fn commit(&mut self, batch: BTreeMap, Vec>) -> Result<(), Error>; +impl Migration for T { + fn version(&self) -> u32 { SimpleMigration::version(self) } + + fn migrate(&self, source: &Database, config: &Config, dest: &mut Database) -> Result<(), Error> { + let mut batch: BTreeMap, Vec> = BTreeMap::new(); + + for (key, value) in source.iter() { + + if let Some((key, value)) = self.simple_migrate(key.to_vec(), value.to_vec()) { + batch.insert(key, value); + } + + if batch.len() == config.batch_size { + try!(commit_batch(dest, &batch)); + batch.clear(); + } + } + + if batch.len() != 0 { + try!(commit_batch(dest, &batch)); + } + + Ok(()) + } } + +/// Commit a batch of writes to a database. +pub fn commit_batch(db: &mut Database, batch: &BTreeMap, Vec>) -> Result<(), Error> { + let transaction = DBTransaction::new(); + + for keypair in batch { + try!(transaction.put(&keypair.0, &keypair.1).map_err(Error::Custom)); + } + + db.write(transaction).map_err(Error::Custom) +} + +/// Get the path where all databases reside. +fn database_path(path: &Path) -> PathBuf { + let mut temp_path = path.to_owned(); + temp_path.pop(); + temp_path +} + +enum TempIndex { + One, + Two, +} + +impl TempIndex { + fn swap(&mut self) { + match *self { + TempIndex::One => *self = TempIndex::Two, + TempIndex::Two => *self = TempIndex::One, + } + } + + // given the path to the old database, get the path of this one. + fn path(&self, db_root: &Path) -> PathBuf { + let mut buf = db_root.to_owned(); + + match *self { + TempIndex::One => buf.push("temp_migration_1"), + TempIndex::Two => buf.push("temp_migration_2"), + }; + + buf + } +} + +/// Manages database migration. +pub struct Manager { + config: Config, + migrations: Vec>, +} + +impl Manager { + /// Creates new migration manager with given configuration. + pub fn new(config: Config) -> Self { + Manager { + config: config, + migrations: vec![], + } + } + + /// Adds new migration rules. + pub fn add_migration(&mut self, migration: T) -> Result<(), Error> where T: Migration { + let version_match = match self.migrations.last() { + Some(last) => last.version() + 1 == migration.version(), + None => true, + }; + + match version_match { + true => Ok(self.migrations.push(Box::new(migration))), + false => Err(Error::CannotAddMigration), + } + } + + /// Performs migration in order, starting with a source path, migrating between two temporary databases, + /// and producing a path where the final migration lives. + pub fn execute(&self, old_path: &Path, version: u32) -> Result { + let migrations = try!(self.migrations_from(version).ok_or(Error::MigrationImpossible)); + let db_config = DatabaseConfig { + prefix_size: None, + max_open_files: 64, + cache_size: None, + compaction: CompactionProfile::default(), + }; + + let db_root = database_path(old_path); + let mut temp_idx = TempIndex::One; + let mut temp_path = temp_idx.path(&db_root); + + // start with the old db. + let old_path_str = try!(old_path.to_str().ok_or(Error::MigrationImpossible)); + let mut cur_db = try!(Database::open(&db_config, old_path_str).map_err(|s| Error::Custom(s))); + for migration in migrations { + // open the target temporary database. + temp_path = temp_idx.path(&db_root); + let temp_path_str = try!(temp_path.to_str().ok_or(Error::MigrationImpossible)); + let mut new_db = try!(Database::open(&db_config, temp_path_str).map_err(|s| Error::Custom(s))); + + // perform the migration from cur_db to new_db. + try!(migration.migrate(&cur_db, &self.config, &mut new_db)); + // next iteration, we will migrate from this db into the other temp. + cur_db = new_db; + temp_idx.swap(); + + // remove the other temporary migration database. + let _ = fs::remove_dir_all(temp_idx.path(&db_root)); + } + Ok(temp_path) + } + + /// Returns true if migration is needed. + pub fn is_needed(&self, version: u32) -> bool { + match self.migrations.last() { + Some(last) => version < last.version(), + None => false, + } + } + + fn migrations_from(&self, version: u32) -> Option<&[Box]> { + // index of the first required migration + let position = self.migrations.iter().position(|m| m.version() == version + 1); + position.map(|p| &self.migrations[p..]) + } +} + diff --git a/util/src/migration/tests.rs b/util/src/migration/tests.rs index b20e18cc9..f441b31e4 100644 --- a/util/src/migration/tests.rs +++ b/util/src/migration/tests.rs @@ -14,19 +14,50 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -use std::collections::BTreeMap; -use migration::{Error, Destination, Migration, Manager, Config}; +//! Tests for migrations. +//! A random temp directory is created. A database is created within it, and migrations +//! are performed in temp sub-directories. -impl Destination for BTreeMap, Vec> { - fn commit(&mut self, batch: BTreeMap, Vec>) -> Result<(), Error> { - self.extend(batch); - Ok(()) +use common::*; +use migration::{Config, SimpleMigration, Manager}; +use kvdb::{Database, DBTransaction}; + +use devtools::RandomTempPath; +use std::path::PathBuf; + +fn db_path(path: &Path) -> PathBuf { + let mut p = path.to_owned(); + p.push("db"); + p +} + +// initialize a database at the given directory with the given values. +fn make_db(path: &Path, pairs: BTreeMap, Vec>) { + let db = Database::open_default(path.to_str().unwrap()).expect("failed to open temp database"); + { + let transaction = DBTransaction::new(); + for (k, v) in pairs { + transaction.put(&k, &v).expect("failed to add pair to transaction"); + } + + db.write(transaction).expect("failed to write db transaction"); + } +} + +// helper for verifying a migrated database. +fn verify_migration(path: &Path, pairs: BTreeMap, Vec>) { + let db = Database::open_default(path.to_str().unwrap()).unwrap(); + + for (k, v) in pairs { + let x = db.get(&k).unwrap().unwrap(); + + assert_eq!(&x[..], &v[..]); } } struct Migration0; -impl Migration for Migration0 { +impl SimpleMigration for Migration0 { fn version(&self) -> u32 { 1 } @@ -42,7 +73,7 @@ impl Migration for Migration0 { struct Migration1; -impl Migration for Migration1 { +impl SimpleMigration for Migration1 { fn version(&self) -> u32 { 2 } @@ -54,68 +85,58 @@ impl Migration for Migration1 { #[test] fn one_simple_migration() { + let dir = RandomTempPath::create_dir(); + let db_path = db_path(dir.as_path()); let mut manager = Manager::new(Config::default()); - let keys = vec![vec![], vec![1u8]]; - let values = vec![vec![], vec![1u8]]; - let db = keys.into_iter().zip(values.into_iter()); + make_db(&db_path, map![vec![] => vec![], vec![1] => vec![1]]); + let expected = map![vec![0x11] => vec![0x22], vec![1, 0x11] => vec![1, 0x22]]; - let expected_keys = vec![vec![0x11u8], vec![1, 0x11]]; - let expected_values = vec![vec![0x22u8], vec![1, 0x22]]; - let expected_db = expected_keys.into_iter().zip(expected_values.into_iter()).collect::>(); - - let mut result = BTreeMap::new(); manager.add_migration(Migration0).unwrap(); - manager.execute(db, 0, &mut result).unwrap(); - assert_eq!(expected_db, result); + let end_path = manager.execute(&db_path, 0).unwrap(); + + verify_migration(&end_path, expected); } #[test] #[should_panic] fn no_migration_needed() { + let dir = RandomTempPath::create_dir(); + let db_path = db_path(dir.as_path()); let mut manager = Manager::new(Config::default()); - let keys = vec![vec![], vec![1u8]]; - let values = vec![vec![], vec![1u8]]; - let db = keys.into_iter().zip(values.into_iter()); + make_db(&db_path, map![vec![] => vec![], vec![1] => vec![1]]); - let mut result = BTreeMap::new(); manager.add_migration(Migration0).unwrap(); - manager.execute(db, 1, &mut result).unwrap(); + manager.execute(&db_path, 1).unwrap(); } #[test] fn multiple_migrations() { + let dir = RandomTempPath::create_dir(); + let db_path = db_path(dir.as_path()); let mut manager = Manager::new(Config::default()); - let keys = vec![vec![], vec![1u8]]; - let values = vec![vec![], vec![1u8]]; - let db = keys.into_iter().zip(values.into_iter()); + make_db(&db_path, map![vec![] => vec![], vec![1] => vec![1]]); + let expected = map![vec![0x11] => vec![], vec![1, 0x11] => vec![]]; - let expected_keys = vec![vec![0x11u8], vec![1, 0x11]]; - let expected_values = vec![vec![], vec![]]; - let expected_db = expected_keys.into_iter().zip(expected_values.into_iter()).collect::>(); - - let mut result = BTreeMap::new(); manager.add_migration(Migration0).unwrap(); manager.add_migration(Migration1).unwrap(); - manager.execute(db, 0, &mut result).unwrap(); - assert_eq!(expected_db, result); + let end_path = manager.execute(&db_path, 0).unwrap(); + + verify_migration(&end_path, expected); } #[test] fn second_migration() { + let dir = RandomTempPath::create_dir(); + let db_path = db_path(dir.as_path()); let mut manager = Manager::new(Config::default()); - let keys = vec![vec![], vec![1u8]]; - let values = vec![vec![], vec![1u8]]; - let db = keys.into_iter().zip(values.into_iter()); + make_db(&db_path, map![vec![] => vec![], vec![1] => vec![1]]); + let expected = map![vec![] => vec![], vec![1] => vec![]]; - let expected_keys = vec![vec![], vec![1u8]]; - let expected_values = vec![vec![], vec![]]; - let expected_db = expected_keys.into_iter().zip(expected_values.into_iter()).collect::>(); - - let mut result = BTreeMap::new(); manager.add_migration(Migration0).unwrap(); manager.add_migration(Migration1).unwrap(); - manager.execute(db, 1, &mut result).unwrap(); - assert_eq!(expected_db, result); + let end_path = manager.execute(&db_path, 1).unwrap(); + + verify_migration(&end_path, expected); } #[test]