Skip to content

Commit

Permalink
Merge pull request nervosnetwork#4343 from chenyukang/yukang-add-migr…
Browse files Browse the repository at this point in the history
…ate-notes

Add --include-background to include background migrations in migrate subcommand
  • Loading branch information
chenyukang authored Feb 20, 2024
2 parents 4cd7845 + 2e29e60 commit 5ba9e04
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 14 deletions.
10 changes: 8 additions & 2 deletions ckb-bin/src/subcommand/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ pub fn migrate(args: MigrateArgs) -> Result<(), ExitCode> {
})?;

if let Some(db) = read_only_db {
let db_status = migrate.check(&db);
// if there are only pending background migrations, they will run automatically
// so here we check with `include_background` as true
let db_status = migrate.check(&db, true);
if matches!(db_status, Ordering::Greater) {
eprintln!(
"The database was created by a higher version CKB executable binary \n\
Expand All @@ -25,8 +27,12 @@ pub fn migrate(args: MigrateArgs) -> Result<(), ExitCode> {
return Err(ExitCode::Failure);
}

// `include_background` is default to false
let db_status = migrate.check(&db, args.include_background);
if args.check {
if matches!(db_status, Ordering::Less) {
// special for bash usage, return 0 means need run migration
// if ckb migrate --check; then ckb migrate --force; fi
return Ok(());
} else {
return Err(ExitCode::Cli);
Expand All @@ -37,7 +43,7 @@ pub fn migrate(args: MigrateArgs) -> Result<(), ExitCode> {
return Ok(());
}

if migrate.require_expensive(&db) && !args.force {
if migrate.require_expensive(&db, args.include_background) && !args.force {
if std::io::stdin().is_terminal() && std::io::stdout().is_terminal() {
let input = prompt("\
\n\
Expand Down
15 changes: 11 additions & 4 deletions db-migration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl Migrations {
/// - Equal: The database version is matched with the executable binary version.
/// - Greater: The database version is greater than the matched version of the executable binary.
/// Requires upgrade the executable binary.
pub fn check(&self, db: &ReadOnlyDB) -> Ordering {
pub fn check(&self, db: &ReadOnlyDB, include_background: bool) -> Ordering {
let db_version = match db
.get_pinned_default(MIGRATION_VERSION_KEY)
.expect("get the version of database")
Expand All @@ -135,9 +135,12 @@ impl Migrations {
};
debug!("Current database version [{}]", db_version);

let latest_version = self
let migrations = self
.migrations
.values()
.filter(|m| include_background || !m.run_in_background());

let latest_version = migrations
.last()
.unwrap_or_else(|| panic!("should have at least one version"))
.version();
Expand All @@ -147,7 +150,7 @@ impl Migrations {
}

/// Check if the migrations will consume a lot of time.
pub fn expensive(&self, db: &ReadOnlyDB) -> bool {
pub fn expensive(&self, db: &ReadOnlyDB, include_background: bool) -> bool {
let db_version = match db
.get_pinned_default(MIGRATION_VERSION_KEY)
.expect("get the version of database")
Expand All @@ -162,8 +165,12 @@ impl Migrations {
}
};

self.migrations
let migrations = self
.migrations
.values()
.filter(|m| include_background || !m.run_in_background());

migrations
.skip_while(|m| m.version() <= db_version.as_str())
.any(|m| m.expensive())
}
Expand Down
5 changes: 2 additions & 3 deletions shared/src/shared_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub fn open_or_create_db(
})?;

if let Some(db) = read_only_db {
match migrate.check(&db) {
match migrate.check(&db, true) {
Ordering::Greater => {
eprintln!(
"The database was created by a higher version CKB executable binary \n\
Expand All @@ -74,8 +74,7 @@ pub fn open_or_create_db(
Ordering::Equal => Ok(RocksDB::open(config, COLUMNS)),
Ordering::Less => {
let can_run_in_background = migrate.can_run_in_background(&db);
eprintln!("can_run_in_background: {}", can_run_in_background);
if migrate.require_expensive(&db) && !can_run_in_background {
if migrate.require_expensive(&db, false) && !can_run_in_background {
eprintln!(
"For optimal performance, CKB recommends migrating your data into a new format.\n\
If you prefer to stick with the older version, \n\
Expand Down
2 changes: 2 additions & 0 deletions util/app-config/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,8 @@ pub struct MigrateArgs {
pub check: bool,
/// Do migration without interactive prompt.
pub force: bool,
/// Whether include background migrations
pub include_background: bool,
}

impl CustomizeSpec {
Expand Down
8 changes: 8 additions & 0 deletions util/app-config/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ pub const ARG_P2P_PORT: &str = "p2p-port";
pub const ARG_RPC_PORT: &str = "rpc-port";
/// Command line argument `--force`.
pub const ARG_FORCE: &str = "force";
/// Command line argument `--include-background`.
pub const ARG_INCLUDE_BACKGROUND: &str = "include-background";
/// Command line argument `--log-to`.
pub const ARG_LOG_TO: &str = "log-to";
/// Command line argument `--bundled`.
Expand Down Expand Up @@ -400,6 +402,12 @@ fn migrate() -> Command {
.conflicts_with(ARG_MIGRATE_CHECK)
.help("Migrate without interactive prompt"),
)
.arg(
Arg::new(ARG_INCLUDE_BACKGROUND)
.long(ARG_INCLUDE_BACKGROUND)
.action(clap::ArgAction::SetTrue)
.help("Whether include background migrations"),
)
}

#[cfg(not(target_os = "windows"))]
Expand Down
2 changes: 2 additions & 0 deletions util/app-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,14 @@ impl Setup {
let config = self.config.into_ckb()?;
let check = matches.get_flag(cli::ARG_MIGRATE_CHECK);
let force = matches.get_flag(cli::ARG_FORCE);
let include_background = matches.get_flag(cli::ARG_INCLUDE_BACKGROUND);

Ok(MigrateArgs {
config,
consensus,
check,
force,
include_background,
})
}

Expand Down
8 changes: 4 additions & 4 deletions util/migrate/src/migrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ impl Migrate {
/// - Equal: The database version is matched with the executable binary version.
/// - Greater: The database version is greater than the matched version of the executable binary.
/// Requires upgrade the executable binary.
pub fn check(&self, db: &ReadOnlyDB) -> Ordering {
self.migrations.check(db)
pub fn check(&self, db: &ReadOnlyDB, include_background: bool) -> Ordering {
self.migrations.check(db, include_background)
}

/// Check whether database requires expensive migrations.
pub fn require_expensive(&self, db: &ReadOnlyDB) -> bool {
self.migrations.expensive(db)
pub fn require_expensive(&self, db: &ReadOnlyDB, include_background: bool) -> bool {
self.migrations.expensive(db, include_background)
}

/// Check whether the pending migrations are all background migrations.
Expand Down
2 changes: 1 addition & 1 deletion util/migrate/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,5 +161,5 @@ fn test_mock_migration() {

let rdb = mg2.open_read_only_db().unwrap().unwrap();

assert_eq!(mg2.check(&rdb), std::cmp::Ordering::Equal)
assert_eq!(mg2.check(&rdb, true), std::cmp::Ordering::Equal)
}

0 comments on commit 5ba9e04

Please sign in to comment.