migration fixes (#1155)

* dont use system temp for migration, do backup

* fixed issue with number of open files on osx

* fixed compilation of util tests

Cargo.lock

@@ -914,7 +914,7 @@ dependencies = [
 [[package]]
 name = "parity-dapps-wallet"
 version = "0.5.0"
-source = "git+https://github.com/ethcore/parity-dapps-wallet-rs.git#37b1691d74ad9bdabc64f58684d9d0a6d1aeef66"
+source = "git+https://github.com/ethcore/parity-dapps-wallet-rs.git#25402ce0a02ae49eb66c9e3852b392267a027ea3"
 dependencies = [
  "parity-dapps 0.3.0 (git+https://github.com/ethcore/parity-dapps-rs.git)",
 ]

parity/migration.rs

@@ -14,13 +14,13 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-use std::{fs, env};
+use std::fs;
 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;
+use util::kvdb::{Database, DatabaseConfig};
 use ethcore::migrations;
 
 /// Database is assumed to be at default version, when no version file is found.
@@ -109,10 +109,19 @@ fn extras_database_path(path: &PathBuf) -> PathBuf {
 }
 
 /// Temporary database path used for migration.
-fn temp_database_path() -> PathBuf {
+fn temp_database_path(path: &PathBuf) -> PathBuf {
-	let mut dir = env::temp_dir();
+	let mut temp_path = path.clone();
-	dir.push("parity_migration");
+	temp_path.pop();
-	dir
+	temp_path.push("temp_migration");
+	temp_path
+}
+
+/// Database backup
+fn backup_database_path(path: &PathBuf) -> PathBuf {
+	let mut backup_path = path.clone();
+	backup_path.pop();
+	backup_path.push("temp_backup");
+	backup_path
 }
 
 /// Default migration settings.
@@ -144,26 +153,40 @@ fn migrate_database(version: u32, path: PathBuf, migrations: MigrationManager) -
 
 	println!("Migrating database {} from version {} to {}", path.to_string_lossy(), version, CURRENT_VERSION);
 
-	// get temp path
+	let temp_path = temp_database_path(&path);
-	let temp_path = temp_database_path();
+	let backup_path = backup_database_path(&path);
 	// remote the dir if it exists
 	let _ = fs::remove_dir_all(&temp_path);
+	let _ = fs::remove_dir_all(&backup_path);
 
 	{
+		let db_config = DatabaseConfig {
+			prefix_size: None,
+			max_open_files: 64,
+		};
+
 		// open old database
-		let old = try!(Database::open_default(path.to_str().unwrap()).map_err(|_| Error::MigrationFailed));
+		let old = try!(Database::open(&db_config, path.to_str().unwrap()).map_err(|_| Error::MigrationFailed));
 
 		// create new database
-		let mut temp = try!(Database::open_default(temp_path.to_str().unwrap()).map_err(|_| Error::MigrationFailed));
+		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));
 	}
 
-	// replace the old database with the new one
+	// create backup
-	try!(fs::remove_dir_all(&path));
+	try!(fs::rename(&path, &backup_path));
-	try!(fs::rename(&temp_path, &path));
 
+	// replace the old database with the new one
+	if let Err(err) = fs::rename(&temp_path, &path) {
+		// if something went wrong, bring back backup
+		try!(fs::rename(&backup_path, path));
+		return Err(From::from(err));
+	}
+
+	// remove backup
+	try!(fs::remove_dir_all(&backup_path));
 	println!("Migration finished");
 
 	Ok(())

util/src/journaldb/archivedb.rs

@@ -47,7 +47,9 @@ impl ArchiveDB {
 	/// Create a new instance from file
 	pub fn new(path: &str) -> ArchiveDB {
 		let opts = DatabaseConfig {
-			prefix_size: Some(12) //use 12 bytes as prefix, this must match account_db prefix
+			//use 12 bytes as prefix, this must match account_db prefix
+			prefix_size: Some(12),
+			max_open_files: 256,
 		};
 		let backing = Database::open(&opts, path).unwrap_or_else(|e| {
 			panic!("Error opening state db: {}", e);

util/src/journaldb/earlymergedb.rs

@@ -77,7 +77,9 @@ impl EarlyMergeDB {
 	/// Create a new instance from file
 	pub fn new(path: &str) -> EarlyMergeDB {
 		let opts = DatabaseConfig {
-			prefix_size: Some(12) //use 12 bytes as prefix, this must match account_db prefix
+			//use 12 bytes as prefix, this must match account_db prefix
+			prefix_size: Some(12),
+			max_open_files: 256,
 		};
 		let backing = Database::open(&opts, path).unwrap_or_else(|e| {
 			panic!("Error opening state db: {}", e);

util/src/journaldb/overlayrecentdb.rs

@@ -107,7 +107,9 @@ impl OverlayRecentDB {
 	/// Create a new instance from file
 	pub fn from_prefs(path: &str) -> OverlayRecentDB {
 		let opts = DatabaseConfig {
-			prefix_size: Some(12) //use 12 bytes as prefix, this must match account_db prefix
+			//use 12 bytes as prefix, this must match account_db prefix
+			prefix_size: Some(12),
+			max_open_files: 256,
 		};
 		let backing = Database::open(&opts, path).unwrap_or_else(|e| {
 			panic!("Error opening state db: {}", e);

util/src/journaldb/refcounteddb.rs

@@ -49,7 +49,9 @@ impl RefCountedDB {
 	/// Create a new instance given a `backing` database.
 	pub fn new(path: &str) -> RefCountedDB {
 		let opts = DatabaseConfig {
-			prefix_size: Some(12) //use 12 bytes as prefix, this must match account_db prefix
+			//use 12 bytes as prefix, this must match account_db prefix
+			prefix_size: Some(12),
+			max_open_files: 256,
 		};
 		let backing = Database::open(&opts, path).unwrap_or_else(|e| {
 			panic!("Error opening state db: {}", e);

util/src/kvdb.rs

@@ -51,7 +51,9 @@ impl DBTransaction {
 /// Database configuration
 pub struct DatabaseConfig {
 	/// Optional prefix size in bytes. Allows lookup by partial key.
-	pub prefix_size: Option<usize>
+	pub prefix_size: Option<usize>,
+	/// Max number of open files.
+	pub max_open_files: i32,
 }
 
 /// Database iterator
@@ -75,13 +77,13 @@ pub struct Database {
 impl Database {
 	/// Open database with default settings.
 	pub fn open_default(path: &str) -> Result<Database, String> {
-		Database::open(&DatabaseConfig { prefix_size: None }, path)
+		Database::open(&DatabaseConfig { prefix_size: None, max_open_files: 256 }, path)
 	}
 
 	/// Open database file. Creates if it does not exist.
 	pub fn open(config: &DatabaseConfig, path: &str) -> Result<Database, String> {
 		let mut opts = Options::new();
-		opts.set_max_open_files(256);
+		opts.set_max_open_files(config.max_open_files);
 		opts.create_if_missing(true);
 		opts.set_use_fsync(false);
 		opts.set_compaction_style(DBCompactionStyle::DBUniversalCompaction);
@@ -203,10 +205,10 @@ mod tests {
 		let path = RandomTempPath::create_dir();
 		let smoke = Database::open_default(path.as_path().to_str().unwrap()).unwrap();
 		assert!(smoke.is_empty());
-		test_db(&DatabaseConfig { prefix_size: None });
+		test_db(&DatabaseConfig { prefix_size: None, max_open_files: 256 });
-		test_db(&DatabaseConfig { prefix_size: Some(1) });
+		test_db(&DatabaseConfig { prefix_size: Some(1), max_open_files: 256 });
-		test_db(&DatabaseConfig { prefix_size: Some(8) });
+		test_db(&DatabaseConfig { prefix_size: Some(8), max_open_files: 256 });
-		test_db(&DatabaseConfig { prefix_size: Some(32) });
+		test_db(&DatabaseConfig { prefix_size: Some(32), max_open_files: 256 });
 	}
 }