From 56b1bf5dc7cb4669a7a18ca5a6d26ca135d8b3a5 Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Wed, 24 Mar 2021 21:13:01 +0800 Subject: [PATCH 1/2] fix: readonly for migrate check --- db-migration/src/lib.rs | 6 ++-- db/src/lib.rs | 2 ++ db/src/read_only_db.rs | 76 +++++++++++++++++++++++++++++++++++++++++ shared/src/shared.rs | 6 ++-- 4 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 db/src/read_only_db.rs diff --git a/db-migration/src/lib.rs b/db-migration/src/lib.rs index be05c5e0f2..8e4ca30939 100644 --- a/db-migration/src/lib.rs +++ b/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") diff --git a/db/src/lib.rs b/db/src/lib.rs index e495bc1776..d291be0980 100644 --- a/db/src/lib.rs +++ b/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; diff --git a/db/src/read_only_db.rs b/db/src/read_only_db.rs new file mode 100644 index 0000000000..aaf3148d41 --- /dev/null +++ b/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, +} + +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>(path: P) -> Result> { + 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> { + 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())) + } +} diff --git a/shared/src/shared.rs b/shared/src/shared.rs index ebec24a4e5..5b4ce11bdc 100644 --- a/shared/src/shared.rs +++ b/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,7 +569,7 @@ 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) @@ -577,7 +577,7 @@ impl SharedBuilder { /// 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) From 160db29c51eee677abf25f4d3e150fd79349d75a Mon Sep 17 00:00:00 2001 From: zhangsoledad <787953403@qq.com> Date: Wed, 24 Mar 2021 21:22:36 +0800 Subject: [PATCH 2/2] fix: migrate incorrect error print --- ckb-bin/src/subcommand/migrate.rs | 2 +- util/app-config/src/args.rs | 2 ++ util/app-config/src/lib.rs | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/ckb-bin/src/subcommand/migrate.rs b/ckb-bin/src/subcommand/migrate.rs index a2ca317fc1..63a8f23acf 100644 --- a/ckb-bin/src/subcommand/migrate.rs +++ b/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 })?; diff --git a/util/app-config/src/args.rs b/util/app-config/src/args.rs index 6e9cb8e38f..28f2d4301b 100644 --- a/util/app-config/src/args.rs +++ b/util/app-config/src/args.rs @@ -176,6 +176,8 @@ pub struct PeerIDArgs { pub struct MigrateArgs { /// The parsed `ckb.toml.` pub config: Box, + /// 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. diff --git a/util/app-config/src/lib.rs b/util/app-config/src/lib.rs index 3b92f304a9..0c126aa19c 100644 --- a/util/app-config/src/lib.rs +++ b/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 { + 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, })