Make rolling session more resilient in case of long finality stalls (#6106)

* Impl dynamic window size. Keep sessions for unfinalized chain Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Stretch also in contructor plus tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * review feedback Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix approval-voting tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * grunting: dispute coordinator tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * add session window column Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * integrate approval vote and fix tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix rolling session tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Small refactor Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * WIP, tests failing Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix approval voting tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix dispute-coordinator tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * remove uneeded param Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fmt Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix loose ends Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * allow failure and tests for it Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix comment Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * comment fix Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * style fix Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * new col doesn't need to be ordered Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fmt and spellcheck Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * db persist tests Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Add v2 config and cols Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * DB upgrade WIP Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * Fix comments Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * add todo Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * update to parity-db to "0.4.2" Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * migration complete Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * One session window size Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix merge damage Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix build errors Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fmt Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * comment fix Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix build Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * make error more explicit Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * add comment Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * refactor conflict merge Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * rename col_data Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * add doc comment Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * fix build Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> * migration: move all cols to v2 Signed-off-by: Andrei Sandu <andrei-mihail@parity.io> Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>
2026-06-23 17:51:09 +00:00 · 2022-11-08 19:16:31 +02:00
parent be4b5fad47
commit c261353380
15 changed files with 944 additions and 291 deletions
@@ -943,7 +943,8 @@ where
 	let parachains_db = open_database(&config.database)?;

 	let approval_voting_config = ApprovalVotingConfig {
-		col_data: parachains_db::REAL_COLUMNS.col_approval_data,
+		col_approval_data: parachains_db::REAL_COLUMNS.col_approval_data,
+		col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data,
 		slot_duration_millis: slot_duration.as_millis() as u64,
 	};

@@ -966,7 +967,8 @@ where
 	};

 	let dispute_coordinator_config = DisputeCoordinatorConfig {
-		col_data: parachains_db::REAL_COLUMNS.col_dispute_coordinator_data,
+		col_dispute_data: parachains_db::REAL_COLUMNS.col_dispute_coordinator_data,
+		col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data,
 	};

 	let rpc_handlers = service::spawn_tasks(service::SpawnTasksParams {
@@ -1537,7 +1539,8 @@ fn revert_chain_selection(db: Arc<dyn Database>, hash: Hash) -> sp_blockchain::R

 fn revert_approval_voting(db: Arc<dyn Database>, hash: Hash) -> sp_blockchain::Result<()> {
 	let config = approval_voting_subsystem::Config {
-		col_data: parachains_db::REAL_COLUMNS.col_approval_data,
+		col_approval_data: parachains_db::REAL_COLUMNS.col_approval_data,
+		col_session_data: parachains_db::REAL_COLUMNS.col_session_window_data,
 		slot_duration_millis: Default::default(),
 	};

@@ -23,6 +23,7 @@ mod upgrade;

 const LOG_TARGET: &str = "parachain::db";

+/// Column configuration per version.
 #[cfg(any(test, feature = "full-node"))]
 pub(crate) mod columns {
 	pub mod v0 {
@@ -31,12 +32,17 @@ pub(crate) mod columns {

 	pub mod v1 {
 		pub const NUM_COLUMNS: u32 = 5;
+	}

+	pub mod v2 {
+		pub const NUM_COLUMNS: u32 = 6;
 		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 COL_SESSION_WINDOW_DATA: u32 = 5;
+
 		pub const ORDERED_COL: &[u32] =
 			&[COL_AVAILABILITY_META, COL_CHAIN_SELECTION_DATA, COL_DISPUTE_COORDINATOR_DATA];
 	}
@@ -56,16 +62,19 @@ pub struct ColumnsConfig {
 	pub col_chain_selection_data: u32,
 	/// The column used by dispute coordinator for data.
 	pub col_dispute_coordinator_data: u32,
+	/// The column used for session window data.
+	pub col_session_window_data: u32,
 }

 /// The real columns used by the parachains DB.
 #[cfg(any(test, feature = "full-node"))]
 pub const REAL_COLUMNS: ColumnsConfig = ColumnsConfig {
-	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,
+	col_availability_data: columns::v2::COL_AVAILABILITY_DATA,
+	col_availability_meta: columns::v2::COL_AVAILABILITY_META,
+	col_approval_data: columns::v2::COL_APPROVAL_DATA,
+	col_chain_selection_data: columns::v2::COL_CHAIN_SELECTION_DATA,
+	col_dispute_coordinator_data: columns::v2::COL_DISPUTE_COORDINATOR_DATA,
+	col_session_window_data: columns::v2::COL_SESSION_WINDOW_DATA,
 };

 #[derive(PartialEq)]
@@ -83,11 +92,18 @@ pub struct CacheSizes {
 	pub availability_meta: usize,
 	/// Cache used by approval data.
 	pub approval_data: usize,
+	/// Cache used by session window data
+	pub session_data: usize,
 }

 impl Default for CacheSizes {
 	fn default() -> Self {
-		CacheSizes { availability_data: 25, availability_meta: 1, approval_data: 5 }
+		CacheSizes {
+			availability_data: 25,
+			availability_meta: 1,
+			approval_data: 5,
+			session_data: 1,
+		}
 	}
 }

@@ -106,17 +122,20 @@ pub fn open_creating_rocksdb(

 	let path = root.join("parachains").join("db");

-	let mut db_config = DatabaseConfig::with_columns(columns::v1::NUM_COLUMNS);
+	let mut db_config = DatabaseConfig::with_columns(columns::v2::NUM_COLUMNS);

 	let _ = db_config
 		.memory_budget
-		.insert(columns::v1::COL_AVAILABILITY_DATA, cache_sizes.availability_data);
+		.insert(columns::v2::COL_AVAILABILITY_DATA, cache_sizes.availability_data);
 	let _ = db_config
 		.memory_budget
-		.insert(columns::v1::COL_AVAILABILITY_META, cache_sizes.availability_meta);
+		.insert(columns::v2::COL_AVAILABILITY_META, cache_sizes.availability_meta);
 	let _ = db_config
 		.memory_budget
-		.insert(columns::v1::COL_APPROVAL_DATA, cache_sizes.approval_data);
+		.insert(columns::v2::COL_APPROVAL_DATA, cache_sizes.approval_data);
+	let _ = db_config
+		.memory_budget
+		.insert(columns::v2::COL_SESSION_WINDOW_DATA, cache_sizes.session_data);

 	let path_str = path
 		.to_str()
@@ -127,7 +146,7 @@ pub fn open_creating_rocksdb(
 	let db = Database::open(&db_config, &path_str)?;
 	let db = polkadot_node_subsystem_util::database::kvdb_impl::DbAdapter::new(
 		db,
-		columns::v1::ORDERED_COL,
+		columns::v2::ORDERED_COL,
 	);

 	Ok(Arc::new(db))
@@ -147,12 +166,12 @@ pub fn open_creating_paritydb(
 	std::fs::create_dir_all(&path_str)?;
 	upgrade::try_upgrade_db(&path, DatabaseKind::ParityDB)?;

-	let db = parity_db::Db::open_or_create(&upgrade::paritydb_version_1_config(&path))
+	let db = parity_db::Db::open_or_create(&upgrade::paritydb_version_2_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::v1::ORDERED_COL,
+		columns::v2::ORDERED_COL,
 	);
 	Ok(Arc::new(db))
 }
@@ -28,7 +28,7 @@ type Version = u32;
 const VERSION_FILE_NAME: &'static str = "parachain_db_version";

 /// Current db version.
-const CURRENT_VERSION: Version = 1;
+const CURRENT_VERSION: Version = 2;

 #[derive(thiserror::Error, Debug)]
 pub enum Error {
@@ -36,7 +36,7 @@ pub enum Error {
 	Io(#[from] io::Error),
 	#[error("The version file format is incorrect")]
 	CorruptedVersionFile,
-	#[error("Future version (expected {current:?}, found {got:?})")]
+	#[error("Parachains DB has a future version (expected {current:?}, found {got:?})")]
 	FutureVersion { current: Version, got: Version },
 }

@@ -56,6 +56,8 @@ pub(crate) fn try_upgrade_db(db_path: &Path, db_kind: DatabaseKind) -> Result<()
 		match get_db_version(db_path)? {
 			// 0 -> 1 migration
 			Some(0) => migrate_from_version_0_to_1(db_path, db_kind)?,
+			// 1 -> 2 migration
+			Some(1) => migrate_from_version_1_to_2(db_path, db_kind)?,
 			// Already at current version, do nothing.
 			Some(CURRENT_VERSION) => (),
 			// This is an arbitrary future version, we don't handle it.
@@ -112,6 +114,19 @@ fn migrate_from_version_0_to_1(path: &Path, db_kind: DatabaseKind) -> Result<(),
 	})
 }

+fn migrate_from_version_1_to_2(path: &Path, db_kind: DatabaseKind) -> Result<(), Error> {
+	gum::info!(target: LOG_TARGET, "Migrating parachains db from version 1 to version 2 ...");
+
+	match db_kind {
+		DatabaseKind::ParityDB => paritydb_migrate_from_version_1_to_2(path),
+		DatabaseKind::RocksDB => rocksdb_migrate_from_version_1_to_2(path),
+	}
+	.and_then(|result| {
+		gum::info!(target: LOG_TARGET, "Migration complete! ");
+		Ok(result)
+	})
+}
+
 /// Migration from version 0 to version 1:
 /// * the number of columns has changed from 3 to 5;
 fn rocksdb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> {
@@ -129,6 +144,22 @@ fn rocksdb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> {
 	Ok(())
 }

+/// Migration from version 1 to version 2:
+/// * the number of columns has changed from 5 to 6;
+fn rocksdb_migrate_from_version_1_to_2(path: &Path) -> Result<(), Error> {
+	use kvdb_rocksdb::{Database, DatabaseConfig};
+
+	let db_path = path
+		.to_str()
+		.ok_or_else(|| super::other_io_error("Invalid database path".into()))?;
+	let db_cfg = DatabaseConfig::with_columns(super::columns::v1::NUM_COLUMNS);
+	let mut db = Database::open(&db_cfg, db_path)?;
+
+	db.add_column()?;
+
+	Ok(())
+}
+
 // This currently clears columns which had their configs altered between versions.
 // The columns to be changed are constrained by the `allowed_columns` vector.
 fn paritydb_fix_columns(
@@ -190,7 +221,18 @@ fn paritydb_fix_columns(
 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 {
+	for i in columns::v2::ORDERED_COL {
+		options.columns[*i as usize].btree_index = true;
+	}
+
+	options
+}
+
+/// Database configuration for version 2.
+pub(crate) fn paritydb_version_2_config(path: &Path) -> parity_db::Options {
+	let mut options =
+		parity_db::Options::with_columns(&path, super::columns::v2::NUM_COLUMNS as u8);
+	for i in columns::v2::ORDERED_COL {
 		options.columns[*i as usize].btree_index = true;
 	}

@@ -202,8 +244,8 @@ pub(crate) fn paritydb_version_1_config(path: &Path) -> parity_db::Options {
 pub(crate) fn paritydb_version_0_config(path: &Path) -> parity_db::Options {
 	let mut options =
 		parity_db::Options::with_columns(&path, super::columns::v1::NUM_COLUMNS as u8);
-	options.columns[super::columns::v1::COL_AVAILABILITY_META as usize].btree_index = true;
-	options.columns[super::columns::v1::COL_CHAIN_SELECTION_DATA as usize].btree_index = true;
+	options.columns[super::columns::v2::COL_AVAILABILITY_META as usize].btree_index = true;
+	options.columns[super::columns::v2::COL_CHAIN_SELECTION_DATA as usize].btree_index = true;

 	options
 }
@@ -218,17 +260,30 @@ fn paritydb_migrate_from_version_0_to_1(path: &Path) -> Result<(), Error> {
 	paritydb_fix_columns(
 		path,
 		paritydb_version_1_config(path),
-		vec![super::columns::v1::COL_DISPUTE_COORDINATOR_DATA],
+		vec![super::columns::v2::COL_DISPUTE_COORDINATOR_DATA],
 	)?;

 	Ok(())
 }

+/// Migration from version 1 to version 2:
+/// - add a new column for session information storage
+fn paritydb_migrate_from_version_1_to_2(path: &Path) -> Result<(), Error> {
+	let mut options = paritydb_version_1_config(path);
+
+	// Adds the session info column.
+	parity_db::Db::add_column(&mut options, Default::default())
+		.map_err(|e| other_io_error(format!("Error adding column {:?}", e)))?;
+
+	Ok(())
+}
+
 #[cfg(test)]
 mod tests {
+	use super::{columns::v2::*, *};
+
 	#[test]
-	fn test_paritydb_migrate_0_1() {
-		use super::{columns::v1::*, *};
+	fn test_paritydb_migrate_0_to_1() {
 		use parity_db::Db;

 		let db_dir = tempfile::tempdir().unwrap();
@@ -246,13 +301,119 @@ mod tests {
 		try_upgrade_db(&path, DatabaseKind::ParityDB).unwrap();

 		let db = Db::open(&paritydb_version_1_config(&path)).unwrap();
+		assert_eq!(db.get(COL_DISPUTE_COORDINATOR_DATA as u8, b"1234").unwrap(), None);
 		assert_eq!(
-			db.get(super::columns::v1::COL_DISPUTE_COORDINATOR_DATA as u8, b"1234").unwrap(),
-			None
-		);
-		assert_eq!(
-			db.get(super::columns::v1::COL_AVAILABILITY_META as u8, b"5678").unwrap(),
+			db.get(COL_AVAILABILITY_META as u8, b"5678").unwrap(),
 			Some("somevalue".as_bytes().to_vec())
 		);
 	}
+
+	#[test]
+	fn test_paritydb_migrate_1_to_2() {
+		use parity_db::Db;
+
+		let db_dir = tempfile::tempdir().unwrap();
+		let path = db_dir.path();
+
+		// We need to properly set db version for upgrade to work.
+		fs::write(version_file_path(path), "1").expect("Failed to write DB version");
+
+		{
+			let db = Db::open_or_create(&paritydb_version_1_config(&path)).unwrap();
+
+			// Write some dummy data
+			db.commit(vec![(
+				COL_DISPUTE_COORDINATOR_DATA as u8,
+				b"1234".to_vec(),
+				Some(b"somevalue".to_vec()),
+			)])
+			.unwrap();
+
+			assert_eq!(db.num_columns(), columns::v1::NUM_COLUMNS as u8);
+		}
+
+		try_upgrade_db(&path, DatabaseKind::ParityDB).unwrap();
+
+		let db = Db::open(&paritydb_version_2_config(&path)).unwrap();
+
+		assert_eq!(db.num_columns(), columns::v2::NUM_COLUMNS as u8);
+
+		assert_eq!(
+			db.get(COL_DISPUTE_COORDINATOR_DATA as u8, b"1234").unwrap(),
+			Some("somevalue".as_bytes().to_vec())
+		);
+
+		// Test we can write the new column.
+		db.commit(vec![(
+			COL_SESSION_WINDOW_DATA as u8,
+			b"1337".to_vec(),
+			Some(b"0xdeadb00b".to_vec()),
+		)])
+		.unwrap();
+
+		// Read back data from new column.
+		assert_eq!(
+			db.get(COL_SESSION_WINDOW_DATA as u8, b"1337").unwrap(),
+			Some("0xdeadb00b".as_bytes().to_vec())
+		);
+	}
+
+	#[test]
+	fn test_rocksdb_migrate_1_to_2() {
+		use kvdb::{DBKey, DBOp};
+		use kvdb_rocksdb::{Database, DatabaseConfig};
+		use polkadot_node_subsystem_util::database::{
+			kvdb_impl::DbAdapter, DBTransaction, KeyValueDB,
+		};
+
+		let db_dir = tempfile::tempdir().unwrap();
+		let db_path = db_dir.path().to_str().unwrap();
+		let db_cfg = DatabaseConfig::with_columns(super::columns::v1::NUM_COLUMNS);
+		let db = Database::open(&db_cfg, db_path).unwrap();
+		assert_eq!(db.num_columns(), super::columns::v1::NUM_COLUMNS as u32);
+
+		// We need to properly set db version for upgrade to work.
+		fs::write(version_file_path(db_dir.path()), "1").expect("Failed to write DB version");
+		{
+			let db = DbAdapter::new(db, columns::v2::ORDERED_COL);
+			db.write(DBTransaction {
+				ops: vec![DBOp::Insert {
+					col: COL_DISPUTE_COORDINATOR_DATA,
+					key: DBKey::from_slice(b"1234"),
+					value: b"0xdeadb00b".to_vec(),
+				}],
+			})
+			.unwrap();
+		}
+
+		try_upgrade_db(&db_dir.path(), DatabaseKind::RocksDB).unwrap();
+
+		let db_cfg = DatabaseConfig::with_columns(super::columns::v2::NUM_COLUMNS);
+		let db = Database::open(&db_cfg, db_path).unwrap();
+
+		assert_eq!(db.num_columns(), super::columns::v2::NUM_COLUMNS);
+
+		let db = DbAdapter::new(db, columns::v2::ORDERED_COL);
+
+		assert_eq!(
+			db.get(COL_DISPUTE_COORDINATOR_DATA, b"1234").unwrap(),
+			Some("0xdeadb00b".as_bytes().to_vec())
+		);
+
+		// Test we can write the new column.
+		db.write(DBTransaction {
+			ops: vec![DBOp::Insert {
+				col: COL_SESSION_WINDOW_DATA,
+				key: DBKey::from_slice(b"1337"),
+				value: b"0xdeadb00b".to_vec(),
+			}],
+		})
+		.unwrap();
+
+		// Read back data from new column.
+		assert_eq!(
+			db.get(COL_SESSION_WINDOW_DATA, b"1337").unwrap(),
+			Some("0xdeadb00b".as_bytes().to_vec())
+		);
+	}
 }