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

[nexus] Improve logging for schema changes #4309

Merged
merged 2 commits into from
Oct 21, 2023
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
29 changes: 20 additions & 9 deletions nexus/db-queries/src/db/datastore/db_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ use std::str::FromStr;

pub const EARLIEST_SUPPORTED_VERSION: &'static str = "1.0.0";

/// Describes a single file containing a schema change, as SQL.
pub struct SchemaUpgradeStep {
pub path: Utf8PathBuf,
pub sql: String,
}

/// Describes a sequence of files containing schema changes.
pub struct SchemaUpgrade {
pub steps: Vec<SchemaUpgradeStep>,
}

/// Reads a "version directory" and reads all SQL changes into
/// a result Vec.
///
Expand All @@ -34,7 +45,7 @@ pub const EARLIEST_SUPPORTED_VERSION: &'static str = "1.0.0";
/// These are sorted lexicographically.
pub async fn all_sql_for_version_migration<P: AsRef<Utf8Path>>(
path: P,
) -> Result<Vec<String>, String> {
) -> Result<SchemaUpgrade, String> {
let target_dir = path.as_ref();
let mut up_sqls = vec![];
let entries = target_dir
Expand All @@ -54,13 +65,12 @@ pub async fn all_sql_for_version_migration<P: AsRef<Utf8Path>>(
}
up_sqls.sort();

let mut result = vec![];
let mut result = SchemaUpgrade { steps: vec![] };
for path in up_sqls.into_iter() {
result.push(
tokio::fs::read_to_string(&path)
.await
.map_err(|e| format!("Cannot read {path}: {e}"))?,
);
let sql = tokio::fs::read_to_string(&path)
.await
.map_err(|e| format!("Cannot read {path}: {e}"))?;
result.steps.push(SchemaUpgradeStep { path: path.to_owned(), sql });
}
Ok(result)
}
Expand Down Expand Up @@ -187,7 +197,8 @@ impl DataStore {
)
.map_err(|e| format!("Invalid schema path: {}", e.display()))?;

let up_sqls = all_sql_for_version_migration(&target_dir).await?;
let schema_change =
all_sql_for_version_migration(&target_dir).await?;

// Confirm the current version, set the "target_version"
// column to indicate that a schema update is in-progress.
Expand All @@ -205,7 +216,7 @@ impl DataStore {
"target_version" => target_version.to_string(),
);

for sql in &up_sqls {
for SchemaUpgradeStep { path: _, sql } in &schema_change.steps {
// Perform the schema change.
self.apply_schema_update(
&current_version,
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-queries/src/db/datastore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ mod zpool;

pub use address_lot::AddressLotCreateResult;
pub use db_metadata::{
all_sql_for_version_migration, EARLIEST_SUPPORTED_VERSION,
all_sql_for_version_migration, SchemaUpgrade, SchemaUpgradeStep,
EARLIEST_SUPPORTED_VERSION,
};
pub use dns::DnsVersionUpdateBuilder;
pub use instance::InstanceAndActiveVmm;
Expand Down
15 changes: 11 additions & 4 deletions nexus/tests/integration_tests/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ async fn apply_update_as_transaction(
match apply_update_as_transaction_inner(client, sql).await {
Ok(()) => break,
Err(err) => {
warn!(log, "Failed to apply update as transaction"; "err" => err.to_string());
client
.batch_execute("ROLLBACK;")
.await
Expand All @@ -111,7 +112,9 @@ async fn apply_update(
version: &str,
times_to_apply: usize,
) {
info!(log, "Performing upgrade to {version}");
let log = log.new(o!("target version" => version.to_string()));
info!(log, "Performing upgrade");

let client = crdb.connect().await.expect("failed to connect");

// We skip this for the earliest supported version because these tables
Expand All @@ -126,11 +129,15 @@ async fn apply_update(
}

let target_dir = Utf8PathBuf::from(SCHEMA_DIR).join(version);
let sqls = all_sql_for_version_migration(&target_dir).await.unwrap();
let schema_change =
all_sql_for_version_migration(&target_dir).await.unwrap();

for _ in 0..times_to_apply {
for sql in sqls.iter() {
apply_update_as_transaction(log, &client, sql).await;
for nexus_db_queries::db::datastore::SchemaUpgradeStep { path, sql } in
&schema_change.steps
{
info!(log, "Applying sql schema upgrade step"; "path" => path.to_string());
apply_update_as_transaction(&log, &client, sql).await;
}
}

Expand Down