review + proper version constants

Signed-off-by: Andrei Sandu <[email protected]>

node/service/src/parachains_db/mod.rs

@@ -28,15 +28,18 @@ pub(crate) mod columns {
 	pub mod v0 {
 		pub const NUM_COLUMNS: u32 = 3;
 	}
-	pub const NUM_COLUMNS: u32 = 5;
-
-	pub const COL_AVAILABILITY_DATA: u32 = 0;
-	pub const COL_AVAILABILITY_META: u32 = 1;
-	pub const COL_APPROVAL_DATA: u32 = 2;
-	pub const COL_CHAIN_SELECTION_DATA: u32 = 3;
-	pub const COL_DISPUTE_COORDINATOR_DATA: u32 = 4;
-	pub const ORDERED_COL: &[u32] =
-		&[COL_AVAILABILITY_META, COL_CHAIN_SELECTION_DATA, COL_DISPUTE_COORDINATOR_DATA];
+
+	pub mod v1 {
+		pub const NUM_COLUMNS: u32 = 5;
+
+		pub const COL_AVAILABILITY_DATA: u32 = 0;
+		pub const COL_AVAILABILITY_META: u32 = 1;
+		pub const COL_APPROVAL_DATA: u32 = 2;
+		pub const COL_CHAIN_SELECTION_DATA: u32 = 3;
+		pub const COL_DISPUTE_COORDINATOR_DATA: u32 = 4;
+		pub const ORDERED_COL: &[u32] =
+			&[COL_AVAILABILITY_META, COL_CHAIN_SELECTION_DATA, COL_DISPUTE_COORDINATOR_DATA];
+	}
 }
 
 /// Columns used by different subsystems.
@@ -58,11 +61,11 @@ pub struct ColumnsConfig {
 /// The real columns used by the parachains DB.
 #[cfg(any(test, feature = "full-node"))]
 pub const REAL_COLUMNS: ColumnsConfig = ColumnsConfig {
-	col_availability_data: columns::COL_AVAILABILITY_DATA,
-	col_availability_meta: columns::COL_AVAILABILITY_META,
-	col_approval_data: columns::COL_APPROVAL_DATA,
-	col_chain_selection_data: columns::COL_CHAIN_SELECTION_DATA,
-	col_dispute_coordinator_data: columns::COL_DISPUTE_COORDINATOR_DATA,
+	col_availability_data: columns::v1::COL_AVAILABILITY_DATA,
+	col_availability_meta: columns::v1::COL_AVAILABILITY_META,
+	col_approval_data: columns::v1::COL_APPROVAL_DATA,
+	col_chain_selection_data: columns::v1::COL_CHAIN_SELECTION_DATA,
+	col_dispute_coordinator_data: columns::v1::COL_DISPUTE_COORDINATOR_DATA,
 };
 
 #[derive(PartialEq)]
@@ -103,17 +106,17 @@ pub fn open_creating_rocksdb(
 
 	let path = root.join("parachains").join("db");
 
-	let mut db_config = DatabaseConfig::with_columns(columns::NUM_COLUMNS);
+	let mut db_config = DatabaseConfig::with_columns(columns::v1::NUM_COLUMNS);
 
 	let _ = db_config
 		.memory_budget
-		.insert(columns::COL_AVAILABILITY_DATA, cache_sizes.availability_data);
+		.insert(columns::v1::COL_AVAILABILITY_DATA, cache_sizes.availability_data);
 	let _ = db_config
 		.memory_budget
-		.insert(columns::COL_AVAILABILITY_META, cache_sizes.availability_meta);
+		.insert(columns::v1::COL_AVAILABILITY_META, cache_sizes.availability_meta);
 	let _ = db_config
 		.memory_budget
-		.insert(columns::COL_APPROVAL_DATA, cache_sizes.approval_data);
+		.insert(columns::v1::COL_APPROVAL_DATA, cache_sizes.approval_data);
 
 	let path_str = path
 		.to_str()
@@ -122,8 +125,10 @@ pub fn open_creating_rocksdb(
 	std::fs::create_dir_all(&path_str)?;
 	upgrade::try_upgrade_db(&path, DatabaseKind::RocksDB)?;
 	let db = Database::open(&db_config, &path_str)?;
-	let db =
-		polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new(db, columns::ORDERED_COL);
+	let db = polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new(
+		db,
+		columns::v1::ORDERED_COL,
+	);
 
 	Ok(Arc::new(db))
 }
@@ -142,17 +147,12 @@ pub fn open_creating_paritydb(
 	std::fs::create_dir_all(&path_str)?;
 	upgrade::try_upgrade_db(&path, DatabaseKind::ParityDB)?;
 
-	let mut options = parity_db::Options::with_columns(&path, columns::NUM_COLUMNS as u8);
-	for i in columns::ORDERED_COL {
-		options.columns[*i as usize].btree_index = true;
-	}
-
-	let db = parity_db::Db::open_or_create(&options)
+	let db = parity_db::Db::open_or_create(&upgrade::paritydb_version_1_config(&path))
 		.map_err(|err| io::Error::new(io::ErrorKind::Other, format!("{:?}", err)))?;
 
 	let db = polkadot_node_subsystem_util::database::paritydb_impl::DbAdapter::new(
 		db,
-		columns::ORDERED_COL,
+		columns::v1::ORDERED_COL,
 	);
 	Ok(Arc::new(db))
 }

node/service/src/parachains_db/upgrade.rs

@@ -66,8 +66,7 @@ pub(crate) fn try_upgrade_db(db_path: &Path, db_kind: DatabaseKind) -> Result<()
 			// We handle this as a `0 -> 1` migration.
 			None if db_kind == DatabaseKind::ParityDB =>
 				migrate_from_version_0_to_1(db_path, db_kind)?,
-			// Fresh db.
-			None => (),
+			None => unreachable!(),
 		}
 	}
 
@@ -101,12 +100,16 @@ fn version_file_path(path: &Path) -> PathBuf {
 }
 
 fn migrate_from_version_0_to_1(path: &Path, db_kind: DatabaseKind) -> Result<(), Error> {
-	gum::info!(target: LOG_TARGET, "Migrating parachains db from version 0 to version 1",);
+	gum::info!(target: LOG_TARGET, "Migrating parachains db from version 0 to version 1 ...");
 
 	match db_kind {
 		DatabaseKind::ParityDB => paritydb_migrate_from_version_0_to_1(path),
 		DatabaseKind::RocksDB => rocksdb_migrate_from_version_0_to_1(path),
 	}
+	.and_then(|result| {
+		gum::info!(target: LOG_TARGET, "Migration complete! ");
+		Ok(result)
+	})
 }
 
 /// Migration from version 0 to version 1:
@@ -135,7 +138,9 @@ fn paritydb_fix_columns(
 ) -> io::Result<()> {
 	// Figure out which columns to delete. This will be determined by inspecting
 	// the metadata file.
-	if let Ok(Some(metadata)) = parity_db::Options::load_metadata(&path) {
+	if let Some(metadata) = parity_db::Options::load_metadata(&path)
+		.map_err(|e| other_io_error(format!("Error reading metadata {:?}", e)))?
+	{
 		let columns_to_clear = metadata
 			.columns
 			.into_iter()
@@ -181,22 +186,28 @@ fn paritydb_fix_columns(
 	Ok(())
 }
 
+/// Database configuration for version 1.
+pub(crate) fn paritydb_version_1_config(path: &Path) -> parity_db::Options {
+	let mut options =
+		parity_db::Options::with_columns(&path, super::columns::v1::NUM_COLUMNS as u8);
+	for i in columns::v1::ORDERED_COL {
+		options.columns[*i as usize].btree_index = true;
+	}
+
+	options
+}
+
 /// Migration from version 0 to version 1.
 /// Cases covered:
 /// - upgrading from v0.9.23 or earlier -> the `dispute coordinator column` was changed
 /// - upgrading from v0.9.24+ -> this is a no op assuming the DB has been manually fixed as per
 /// release notes
 fn paritydb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> {
-	let mut options = parity_db::Options::with_columns(&path, super::columns::NUM_COLUMNS as u8);
-	for i in columns::ORDERED_COL {
-		options.columns[*i as usize].btree_index = true;
-	}
-
 	// Delete the `dispute coordinator` column if needed (if column configuration is changed).
 	paritydb_fix_columns(
-		&path,
-		options.clone(),
-		vec![super::columns::COL_DISPUTE_COORDINATOR_DATA],
+		path,
+		paritydb_version_1_config(path),
+		vec![super::columns::v1::COL_DISPUTE_COORDINATOR_DATA],
 	)?;
 
 	Ok(())