Merge #2629

2629: readonly for migrate check r=quake,yangby-cryptape a=zhangsoledad

* Perform migration check with read-only mode to prevent automatically create columns breaking compatibility
* Fix the error message is displayed incorrectly while performing the migration
   Run error: Error { kind: Spec, inner: GenesisMismatch(...

Co-authored-by: zhangsoledad <787953403@qq.com>

ckb-bin/src/subcommand/migrate.rs

@@ -41,7 +41,7 @@ pub fn migrate(args: MigrateArgs, async_handle: Handle) -> Result<(), ExitCode>
         }
     }
 
-    let (_shared, _table) = builder.build().map_err(|err| {
+    let (_shared, _table) = builder.consensus(args.consensus).build().map_err(|err| {
         eprintln!("Run error: {:?}", err);
         ExitCode::Failure
     })?;

db-migration/src/lib.rs

@@ -1,5 +1,5 @@
 //! TODO(doc): @quake
-use ckb_db::RocksDB;
+use ckb_db::{ReadOnlyDB, RocksDB};
 use ckb_db_schema::MIGRATION_VERSION_KEY;
 use ckb_error::{Error, InternalErrorKind};
 use ckb_logger::{error, info};
@@ -35,7 +35,7 @@ impl Migrations {
     /// Check whether database requires migration
     ///
     /// Return true if migration is required
-    pub fn check(&self, db: &RocksDB) -> bool {
+    pub fn check(&self, db: &ReadOnlyDB) -> bool {
         let db_version = match db
             .get_pinned_default(MIGRATION_VERSION_KEY)
             .expect("get the version of database")
@@ -54,7 +54,7 @@ impl Migrations {
     }
 
     /// Check if the migrations will consume a lot of time.
-    pub fn expensive(&self, db: &RocksDB) -> bool {
+    pub fn expensive(&self, db: &ReadOnlyDB) -> bool {
         let db_version = match db
             .get_pinned_default(MIGRATION_VERSION_KEY)
             .expect("get the version of database")

db/src/lib.rs

@@ -8,12 +8,14 @@ use std::{fmt, result};
 
 pub mod db;
 pub mod iter;
+pub mod read_only_db;
 pub mod snapshot;
 pub mod transaction;
 pub mod write_batch;
 
 pub use crate::db::RocksDB;
 pub use crate::iter::DBIterator;
+pub use crate::read_only_db::ReadOnlyDB;
 pub use crate::snapshot::RocksDBSnapshot;
 pub use crate::transaction::{RocksDBTransaction, RocksDBTransactionSnapshot};
 pub use crate::write_batch::RocksDBWriteBatch;

db/src/read_only_db.rs

@@ -0,0 +1,76 @@
+//! ReadOnlyDB wrapper base on rocksdb read_only_open mode
+use crate::{internal_error, Result};
+use ckb_logger::info;
+use rocksdb::ops::{GetPinned, Open};
+use rocksdb::{DBPinnableSlice, Options, ReadOnlyDB as RawReadOnlyDB};
+use std::path::Path;
+use std::sync::Arc;
+
+/// ReadOnlyDB wrapper
+pub struct ReadOnlyDB {
+    pub(crate) inner: Arc<RawReadOnlyDB>,
+}
+
+impl ReadOnlyDB {
+    /// The behavior is similar to DB::Open,
+    /// except that it opens DB in read-only mode.
+    /// One big difference is that when opening the DB as read-only,
+    /// you don't need to specify all Column Families
+    /// -- you can only open a subset of Column Families.
+    pub fn open<P: AsRef<Path>>(path: P) -> Result<Option<Self>> {
+        let opts = Options::default();
+        RawReadOnlyDB::open(&opts, path).map_or_else(
+            |err| {
+                let err_str = err.as_ref();
+                // notice: err msg difference
+                if err_str.starts_with("NotFound")
+                    && err_str.ends_with("does not exist")
+                {
+                    Ok(None)
+                } else if err_str.starts_with("Corruption:") {
+                    info!(
+                        "DB corrupted: {}.\n\
+                        Try ckb db-repair command to repair DB.\n\
+                        Note: Currently there is a limitation that un-flushed column families will be lost after repair.\
+                        This would happen even if the DB is in healthy state.\n\
+                        See https://github.com/facebook/rocksdb/wiki/RocksDB-Repairer for detail",
+                        err_str
+                    );
+                    Err(internal_error("DB corrupted"))
+                } else {
+                    Err(internal_error(format!(
+                        "failed to open the database: {}",
+                        err
+                    )))
+                }
+            },
+            |db| {
+                Ok(Some(ReadOnlyDB {
+                    inner: Arc::new(db),
+                }))
+            },
+        )
+    }
+
+    /// Return the value associated with a key using RocksDB's PinnableSlice from the default column
+    /// so as to avoid unnecessary memory copy.
+    pub fn get_pinned_default(&self, key: &[u8]) -> Result<Option<DBPinnableSlice>> {
+        self.inner.get_pinned(&key).map_err(internal_error)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::ReadOnlyDB;
+
+    #[test]
+    fn test_open_read_only_not_exist() {
+        let tmp_dir = tempfile::Builder::new()
+            .prefix("test_open_read_only_not_exist")
+            .tempdir()
+            .unwrap();
+
+        let db = ReadOnlyDB::open(&tmp_dir);
+        assert!(matches!(db, Ok(x) if x.is_none()))
+    }
+}

shared/src/shared.rs

@@ -7,7 +7,7 @@ use ckb_chain_spec::consensus::Consensus;
 use ckb_chain_spec::SpecError;
 use ckb_constant::store::TX_INDEX_UPPER_BOUND;
 use ckb_constant::sync::MAX_TIP_AGE;
-use ckb_db::{Direction, IteratorMode, RocksDB};
+use ckb_db::{Direction, IteratorMode, ReadOnlyDB, RocksDB};
 use ckb_db_migration::{DefaultMigration, Migrations};
 use ckb_db_schema::COLUMN_BLOCK_BODY;
 use ckb_db_schema::{COLUMNS, COLUMN_NUMBER_HASH};
@@ -569,15 +569,15 @@ impl SharedBuilder {
     ///
     /// Return true if migration is required
     pub fn migration_check(&self) -> bool {
-        RocksDB::prepare_for_bulk_load_open(&self.db_config.path, COLUMNS)
+        ReadOnlyDB::open(&self.db_config.path)
             .unwrap_or_else(|err| panic!("{}", err))
             .map(|db| self.migrations.check(&db))
             .unwrap_or(false)
     }
 
     /// Check whether database requires expensive migrations.
     pub fn require_expensive_migrations(&self) -> bool {
-        RocksDB::prepare_for_bulk_load_open(&self.db_config.path, COLUMNS)
+        ReadOnlyDB::open(&self.db_config.path)
             .unwrap_or_else(|err| panic!("{}", err))
             .map(|db| self.migrations.expensive(&db))
             .unwrap_or(false)

util/app-config/src/args.rs

@@ -176,6 +176,8 @@ pub struct PeerIDArgs {
 pub struct MigrateArgs {
     /// The parsed `ckb.toml.`
     pub config: Box<CKBAppConfig>,
+    /// Loaded consensus.
+    pub consensus: Consensus,
     /// Check whether it is required to do migration instead of really perform the migration.
     pub check: bool,
     /// Do migration without interactive prompt.

util/app-config/src/lib.rs

@@ -97,12 +97,14 @@ impl Setup {
 
     /// `migrate` subcommand has one `flags` arg, trigger this arg with "--check"
     pub fn migrate<'m>(self, matches: &ArgMatches<'m>) -> Result<MigrateArgs, ExitCode> {
+        let consensus = self.consensus()?;
         let config = self.config.into_ckb()?;
         let check = matches.is_present(cli::ARG_MIGRATE_CHECK);
         let force = matches.is_present(cli::ARG_FORCE);
 
         Ok(MigrateArgs {
             config,
+            consensus,
             check,
             force,
         })