Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport 114: Add --include-background to include background migrations in migrate subcommand #4358

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
}
Loading