From 72369bb9726d6777266df148d6a2c90bba94833c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 17 Jun 2024 15:32:28 -0700 Subject: [PATCH] Plumb log --- dev-tools/omdb/src/bin/omdb/db.rs | 3 +- nexus/db-queries/src/db/collection_attach.rs | 20 ++++----- nexus/db-queries/src/db/collection_detach.rs | 12 +++--- .../src/db/collection_detach_many.rs | 16 +++---- nexus/db-queries/src/db/collection_insert.rs | 4 +- .../src/db/datastore/db_metadata.rs | 6 +-- nexus/db-queries/src/db/datastore/mod.rs | 4 +- .../src/db/datastore/pub_test_utils.rs | 2 +- nexus/db-queries/src/db/explain.rs | 4 +- nexus/db-queries/src/db/pagination.rs | 8 ++-- nexus/db-queries/src/db/pool.rs | 22 ++++++---- nexus/db-queries/src/db/pool_connection.rs | 42 ++++++++++++++++--- .../db-queries/src/db/queries/external_ip.rs | 5 ++- nexus/db-queries/src/db/queries/next_item.rs | 6 ++- .../src/db/queries/region_allocation.rs | 2 +- .../virtual_provisioning_collection_update.rs | 8 ++-- nexus/db-queries/src/db/queries/vpc_subnet.rs | 3 +- nexus/db-queries/src/db/saga_recovery.rs | 2 +- nexus/src/bin/schema-updater.rs | 2 +- nexus/src/context.rs | 7 +++- nexus/src/populate.rs | 7 +++- nexus/tests/integration_tests/schema.rs | 1 + 22 files changed, 118 insertions(+), 68 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index 25179fe4ef..668905f74c 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -231,7 +231,8 @@ impl DbUrlOptions { eprintln!("note: using database URL {}", &db_url); let db_config = db::Config { url: db_url.clone() }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&db_config)); + let pool = + Arc::new(db::Pool::new_qorb_single_host(&log.clone(), &db_config)); // Being a dev tool, we want to try this operation even if the schema // doesn't match what we expect. So we use `DataStore::new_unchecked()` diff --git a/nexus/db-queries/src/db/collection_attach.rs b/nexus/db-queries/src/db/collection_attach.rs index 7fdaec5aff..544054792b 100644 --- a/nexus/db-queries/src/db/collection_attach.rs +++ b/nexus/db-queries/src/db/collection_attach.rs @@ -857,7 +857,7 @@ mod test { dev::test_setup_log("test_attach_missing_collection_fails"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -886,7 +886,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_missing_resource_fails"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -923,7 +923,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_once"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -971,7 +971,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_once_synchronous"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -1020,7 +1020,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_multiple_times"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -1076,7 +1076,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_beyond_capacity_fails"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -1140,7 +1140,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_while_already_attached"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -1247,7 +1247,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_once"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -1302,7 +1302,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_deleted_resource_fails"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -1347,7 +1347,7 @@ mod test { let logctx = dev::test_setup_log("test_attach_without_update_filter"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; diff --git a/nexus/db-queries/src/db/collection_detach.rs b/nexus/db-queries/src/db/collection_detach.rs index 661eaed6d0..2cb5afbfe8 100644 --- a/nexus/db-queries/src/db/collection_detach.rs +++ b/nexus/db-queries/src/db/collection_detach.rs @@ -784,7 +784,7 @@ mod test { dev::test_setup_log("test_detach_missing_collection_fails"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -812,7 +812,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_missing_resource_fails"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -848,7 +848,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_once"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -888,7 +888,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_while_already_detached"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -952,7 +952,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_deleted_resource_fails"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -996,7 +996,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_without_update_filter"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; diff --git a/nexus/db-queries/src/db/collection_detach_many.rs b/nexus/db-queries/src/db/collection_detach_many.rs index 8229f7736b..fc2c342936 100644 --- a/nexus/db-queries/src/db/collection_detach_many.rs +++ b/nexus/db-queries/src/db/collection_detach_many.rs @@ -776,7 +776,7 @@ mod test { dev::test_setup_log("test_detach_missing_collection_fails"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -806,7 +806,7 @@ mod test { dev::test_setup_log("test_detach_missing_resource_succeeds"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -847,7 +847,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_once"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -890,7 +890,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_once_synchronous"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -935,7 +935,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_while_already_detached"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -991,7 +991,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_filter_collection"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -1042,7 +1042,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_deleted_resource"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -1100,7 +1100,7 @@ mod test { let logctx = dev::test_setup_log("test_detach_many"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; diff --git a/nexus/db-queries/src/db/collection_insert.rs b/nexus/db-queries/src/db/collection_insert.rs index f3bb6602ac..6d125a6d1a 100644 --- a/nexus/db-queries/src/db/collection_insert.rs +++ b/nexus/db-queries/src/db/collection_insert.rs @@ -558,7 +558,7 @@ mod test { let logctx = dev::test_setup_log("test_collection_not_present"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; @@ -588,7 +588,7 @@ mod test { let logctx = dev::test_setup_log("test_collection_present"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = setup_db(&pool).await; diff --git a/nexus/db-queries/src/db/datastore/db_metadata.rs b/nexus/db-queries/src/db/datastore/db_metadata.rs index f4c23b5cce..c375faa2e1 100644 --- a/nexus/db-queries/src/db/datastore/db_metadata.rs +++ b/nexus/db-queries/src/db/datastore/db_metadata.rs @@ -511,7 +511,7 @@ mod test { let mut crdb = test_db::test_setup_database(&logctx.log).await; let cfg = db::Config { url: crdb.pg_config().clone() }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let datastore = Arc::new(DataStore::new(&logctx.log, pool, None).await.unwrap()); @@ -559,7 +559,7 @@ mod test { let mut crdb = test_db::test_setup_database(&logctx.log).await; let cfg = db::Config { url: crdb.pg_config().clone() }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let conn = pool.claim().await.unwrap(); // Mimic the layout of "schema/crdb". @@ -671,7 +671,7 @@ mod test { let mut crdb = test_db::test_setup_database(&logctx.log).await; let cfg = db::Config { url: crdb.pg_config().clone() }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let conn = pool.claim().await.unwrap(); // Mimic the layout of "schema/crdb". diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 880be8a90a..add420add2 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -1573,7 +1573,7 @@ mod test { dev::test_setup_log("test_queries_do_not_require_full_table_scan"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let datastore = DataStore::new(&logctx.log, Arc::new(pool), None).await.unwrap(); let conn = datastore.pool_connection_for_tests().await.unwrap(); @@ -1622,7 +1622,7 @@ mod test { let logctx = dev::test_setup_log("test_sled_ipv6_address_allocation"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let datastore = Arc::new(DataStore::new(&logctx.log, pool, None).await.unwrap()); let opctx = OpContext::for_tests( diff --git a/nexus/db-queries/src/db/datastore/pub_test_utils.rs b/nexus/db-queries/src/db/datastore/pub_test_utils.rs index 78e8bf8b31..98a58ebb10 100644 --- a/nexus/db-queries/src/db/datastore/pub_test_utils.rs +++ b/nexus/db-queries/src/db/datastore/pub_test_utils.rs @@ -29,7 +29,7 @@ pub async fn datastore_test( use crate::authn; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let datastore = Arc::new(DataStore::new(&logctx.log, pool, None).await.unwrap()); diff --git a/nexus/db-queries/src/db/explain.rs b/nexus/db-queries/src/db/explain.rs index 9e050a1da1..c3e7961335 100644 --- a/nexus/db-queries/src/db/explain.rs +++ b/nexus/db-queries/src/db/explain.rs @@ -144,7 +144,7 @@ mod test { let logctx = dev::test_setup_log("test_explain_async"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = pool.claim().await.unwrap(); create_schema(&pool).await; @@ -169,7 +169,7 @@ mod test { let logctx = dev::test_setup_log("test_explain_full_table_scan"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = pool.claim().await.unwrap(); create_schema(&pool).await; diff --git a/nexus/db-queries/src/db/pagination.rs b/nexus/db-queries/src/db/pagination.rs index 24d6971e08..6d4253c055 100644 --- a/nexus/db-queries/src/db/pagination.rs +++ b/nexus/db-queries/src/db/pagination.rs @@ -402,7 +402,7 @@ mod test { dev::test_setup_log("test_paginated_single_column_ascending"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); use schema::test_users::dsl; @@ -437,7 +437,7 @@ mod test { dev::test_setup_log("test_paginated_single_column_descending"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); use schema::test_users::dsl; @@ -472,7 +472,7 @@ mod test { dev::test_setup_log("test_paginated_multicolumn_ascending"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); use schema::test_users::dsl; @@ -526,7 +526,7 @@ mod test { dev::test_setup_log("test_paginated_multicolumn_descending"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = db::Pool::new_qorb_single_host(&cfg); + let pool = db::Pool::new_qorb_single_host(&logctx.log, &cfg); use schema::test_users::dsl; diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index adf7778e93..5dcc4a7d46 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -13,6 +13,7 @@ use qorb::policy::Policy; use qorb::resolver::{AllBackends, Resolver}; use qorb::resolvers::dns::{DnsResolver, DnsResolverConfig}; use qorb::service; +use slog::Logger; use std::collections::BTreeMap; use std::net::SocketAddr; use std::sync::Arc; @@ -73,7 +74,9 @@ fn make_single_host_resolver( Box::new(SingleHostResolver::new(config)) } -fn make_postgres_connector() -> qorb::backend::SharedConnector { +fn make_postgres_connector( + log: &Logger, +) -> qorb::backend::SharedConnector { // Create postgres connections. // // We're currently relying on the DieselPgConnector doing the following: @@ -82,7 +85,7 @@ fn make_postgres_connector() -> qorb::backend::SharedConnector { let user = "root"; let db = "omicron"; let args = Some("sslmode=disable"); - Arc::new(DieselPgConnector::new(user, db, args)) + Arc::new(DieselPgConnector::new(log, user, db, args)) } impl Pool { @@ -90,12 +93,12 @@ impl Pool { /// /// Creating this pool does not necessarily wait for connections to become /// available, as backends may shift over time. - pub fn new_qorb(bootstrap_dns: Vec) -> Self { + pub fn new_qorb(log: &Logger, bootstrap_dns: Vec) -> Self { // Make sure diesel-dtrace's USDT probes are enabled. usdt::register_probes().expect("Failed to register USDT DTrace probes"); let resolver = make_dns_resolver(bootstrap_dns); - let connector = make_postgres_connector(); + let connector = make_postgres_connector(log); let policy = Policy::default(); Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) } @@ -108,12 +111,12 @@ impl Pool { /// on a single instance of the database. /// /// In production, [Self::new_qorb] should be preferred. - pub fn new_qorb_single_host(db_config: &DbConfig) -> Self { + pub fn new_qorb_single_host(log: &Logger, db_config: &DbConfig) -> Self { // Make sure diesel-dtrace's USDT probes are enabled. usdt::register_probes().expect("Failed to register USDT DTrace probes"); let resolver = make_single_host_resolver(db_config); - let connector = make_postgres_connector(); + let connector = make_postgres_connector(log); let policy = Policy::default(); Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) } @@ -123,12 +126,15 @@ impl Pool { /// if claims are not quickly available. /// /// This is intended for test-only usage. - pub fn new_qorb_single_host_failfast(db_config: &DbConfig) -> Self { + pub fn new_qorb_single_host_failfast( + log: &Logger, + db_config: &DbConfig, + ) -> Self { // Make sure diesel-dtrace's USDT probes are enabled. usdt::register_probes().expect("Failed to register USDT DTrace probes"); let resolver = make_single_host_resolver(db_config); - let connector = make_postgres_connector(); + let connector = make_postgres_connector(log); let policy = Policy { claim_timeout: tokio::time::Duration::from_millis(1), diff --git a/nexus/db-queries/src/db/pool_connection.rs b/nexus/db-queries/src/db/pool_connection.rs index fdf78c99dd..04b884db71 100644 --- a/nexus/db-queries/src/db/pool_connection.rs +++ b/nexus/db-queries/src/db/pool_connection.rs @@ -12,6 +12,7 @@ use diesel::Connection; use diesel::PgConnection; use diesel_dtrace::DTraceConnection; use qorb::backend::{self, Backend, Error}; +use slog::Logger; pub type DbConnection = DTraceConnection; @@ -20,6 +21,7 @@ pub const DISALLOW_FULL_TABLE_SCAN_SQL: &str = /// A [backend::Connector] which provides access to [PgConnection]. pub struct DieselPgConnector { + log: Logger, prefix: String, suffix: String, } @@ -35,8 +37,9 @@ impl DieselPgConnector { /// Or, if arguments are supplied: /// /// - postgresql://{user}@{address}/{db}?{args} - pub fn new(user: &str, db: &str, args: Option<&str>) -> Self { + pub fn new(log: &Logger, user: &str, db: &str, args: Option<&str>) -> Self { Self { + log: log.clone(), prefix: format!("postgresql://{user}@"), suffix: format!( "/{db}{}", @@ -70,7 +73,16 @@ impl backend::Connector for DieselPgConnector { Ok::<_, Error>(async_bb8_diesel::Connection::new(pg_conn)) }) .await - .expect("Task panicked establishing connection")?; + .expect("Task panicked establishing connection") + .map_err(|e| { + warn!( + self.log, + "Failed to make connection"; + "error" => e.to_string(), + "backend" => backend.address, + ); + e + })?; Ok(conn) } @@ -78,17 +90,35 @@ impl backend::Connector for DieselPgConnector { &self, conn: &mut Self::Connection, ) -> Result<(), Error> { - conn.batch_execute_async(DISALLOW_FULL_TABLE_SCAN_SQL) - .await - .map_err(|e| Error::Other(anyhow!(e)))?; + conn.batch_execute_async(DISALLOW_FULL_TABLE_SCAN_SQL).await.map_err( + |e| { + warn!( + self.log, + "Failed on_acquire execution"; + "error" => e.to_string() + ); + Error::Other(anyhow!(e)) + }, + )?; Ok(()) } async fn is_valid(&self, conn: &mut Self::Connection) -> Result<(), Error> { let is_broken = conn.is_broken_async().await; if is_broken { + warn!( + self.log, + "Failed is_valid check; connection known to be broken" + ); return Err(Error::Other(anyhow!("Connection broken"))); } - conn.ping_async().await.map_err(|e| Error::Other(anyhow!(e))) + conn.ping_async().await.map_err(|e| { + warn!( + self.log, + "Failed is_valid check; connection failed ping"; + "error" => e.to_string() + ); + Error::Other(anyhow!(e)) + }) } } diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index a01f6646fb..b305e43724 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -917,7 +917,10 @@ mod tests { crate::db::datastore::test_utils::datastore_test(&logctx, &db) .await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg)); + let pool = Arc::new(crate::db::Pool::new_qorb_single_host( + &logctx.log, + &cfg, + )); let db_datastore = Arc::new( crate::db::DataStore::new(&logctx.log, Arc::clone(&pool), None) .await diff --git a/nexus/db-queries/src/db/queries/next_item.rs b/nexus/db-queries/src/db/queries/next_item.rs index e25ff28eba..bacc80274a 100644 --- a/nexus/db-queries/src/db/queries/next_item.rs +++ b/nexus/db-queries/src/db/queries/next_item.rs @@ -708,7 +708,8 @@ mod tests { let log = logctx.log.new(o!()); let mut db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg)); + let pool = + Arc::new(crate::db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let conn = pool.claim().await.unwrap(); // We're going to operate on a separate table, for simplicity. @@ -770,7 +771,8 @@ mod tests { let log = logctx.log.new(o!()); let mut db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg)); + let pool = + Arc::new(crate::db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let conn = pool.claim().await.unwrap(); // We're going to operate on a separate table, for simplicity. diff --git a/nexus/db-queries/src/db/queries/region_allocation.rs b/nexus/db-queries/src/db/queries/region_allocation.rs index 1c15fe68db..3310ff6db6 100644 --- a/nexus/db-queries/src/db/queries/region_allocation.rs +++ b/nexus/db-queries/src/db/queries/region_allocation.rs @@ -497,7 +497,7 @@ mod test { let log = logctx.log.new(o!()); let mut db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = crate::db::Pool::new_qorb_single_host(&cfg); + let pool = crate::db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = pool.claim().await.unwrap(); let volume_id = Uuid::new_v4(); diff --git a/nexus/db-queries/src/db/queries/virtual_provisioning_collection_update.rs b/nexus/db-queries/src/db/queries/virtual_provisioning_collection_update.rs index 775f47a5ec..ea41e7c64e 100644 --- a/nexus/db-queries/src/db/queries/virtual_provisioning_collection_update.rs +++ b/nexus/db-queries/src/db/queries/virtual_provisioning_collection_update.rs @@ -590,7 +590,7 @@ mod test { let log = logctx.log.new(o!()); let mut db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = crate::db::Pool::new_qorb_single_host(&cfg); + let pool = crate::db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = pool.claim().await.unwrap(); let id = Uuid::nil(); @@ -619,7 +619,7 @@ mod test { let log = logctx.log.new(o!()); let mut db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = crate::db::Pool::new_qorb_single_host(&cfg); + let pool = crate::db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = pool.claim().await.unwrap(); let id = Uuid::nil(); @@ -646,7 +646,7 @@ mod test { let log = logctx.log.new(o!()); let mut db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = crate::db::Pool::new_qorb_single_host(&cfg); + let pool = crate::db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = pool.claim().await.unwrap(); let id = Uuid::nil(); @@ -672,7 +672,7 @@ mod test { let log = logctx.log.new(o!()); let mut db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = crate::db::Pool::new_qorb_single_host(&cfg); + let pool = crate::db::Pool::new_qorb_single_host(&logctx.log, &cfg); let conn = pool.claim().await.unwrap(); let id = Uuid::nil(); diff --git a/nexus/db-queries/src/db/queries/vpc_subnet.rs b/nexus/db-queries/src/db/queries/vpc_subnet.rs index 5914561d73..696803217e 100644 --- a/nexus/db-queries/src/db/queries/vpc_subnet.rs +++ b/nexus/db-queries/src/db/queries/vpc_subnet.rs @@ -436,7 +436,8 @@ mod test { let log = logctx.log.new(o!()); let mut db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg)); + let pool = + Arc::new(crate::db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let db_datastore = Arc::new( crate::db::DataStore::new(&log, Arc::clone(&pool), None) .await diff --git a/nexus/db-queries/src/db/saga_recovery.rs b/nexus/db-queries/src/db/saga_recovery.rs index 8a8768ca94..604e445a9f 100644 --- a/nexus/db-queries/src/db/saga_recovery.rs +++ b/nexus/db-queries/src/db/saga_recovery.rs @@ -327,7 +327,7 @@ mod test { ) -> (dev::db::CockroachInstance, Arc) { let db = test_setup_database(&log).await; let cfg = crate::db::Config { url: db.pg_config().clone() }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host(log, &cfg)); let db_datastore = Arc::new( db::DataStore::new(&log, Arc::clone(&pool), None).await.unwrap(), ); diff --git a/nexus/src/bin/schema-updater.rs b/nexus/src/bin/schema-updater.rs index d9ced0e18f..6e2132a75c 100644 --- a/nexus/src/bin/schema-updater.rs +++ b/nexus/src/bin/schema-updater.rs @@ -71,7 +71,7 @@ async fn main() -> anyhow::Result<()> { let log = Logger::root(drain, slog::o!("unit" => "schema_updater")); let crdb_cfg = db::Config { url: args.url }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&crdb_cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host(&log, &crdb_cfg)); let schema_config = SchemaConfig { schema_dir: args.schema_directory }; let all_versions = AllSchemaVersions::load(&schema_config.schema_dir)?; diff --git a/nexus/src/context.rs b/nexus/src/context.rs index 1338625d1a..98eb736712 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -254,11 +254,14 @@ impl ServerContext { let pool = match &config.deployment.database { nexus_config::Database::FromUrl { url } => { info!(log, "Setting up qorb pool from a single host"; "url" => #?url); - db::Pool::new_qorb_single_host(&db::Config { url: url.clone() }) + db::Pool::new_qorb_single_host( + &log, + &db::Config { url: url.clone() }, + ) } nexus_config::Database::FromDns => { info!(log, "Setting up qorb pool from DNS"; "dns_addrs" => #?dns_addrs); - db::Pool::new_qorb(dns_addrs) + db::Pool::new_qorb(&log, dns_addrs) } }; diff --git a/nexus/src/populate.rs b/nexus/src/populate.rs index 3880ec2fed..cbf112dd27 100644 --- a/nexus/src/populate.rs +++ b/nexus/src/populate.rs @@ -380,7 +380,7 @@ mod test { let logctx = dev::test_setup_log("test_populator"); let mut db = test_setup_database(&logctx.log).await; let cfg = db::Config { url: db.pg_config().clone() }; - let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host(&logctx.log, &cfg)); let datastore = Arc::new( db::DataStore::new(&logctx.log, pool, None).await.unwrap(), ); @@ -427,7 +427,10 @@ mod test { // // If we try again with a broken database, we should get a // ServiceUnavailable error, which indicates a transient failure. - let pool = Arc::new(db::Pool::new_qorb_single_host_failfast(&cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host_failfast( + &logctx.log, + &cfg, + )); // We need to create the datastore before tearing down the database, as // it verifies the schema version of the DB while booting. let datastore = Arc::new( diff --git a/nexus/tests/integration_tests/schema.rs b/nexus/tests/integration_tests/schema.rs index 9352bee38a..6520c34dc8 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -955,6 +955,7 @@ async fn dbinit_equals_sum_of_all_up() { // before applying the rest, and grab a connection from that pool. We'll use // it for an extra check later. let pool = nexus_db_queries::db::Pool::new_qorb_single_host( + log, &nexus_db_queries::db::Config { url: crdb.pg_config().clone() }, ); let conn_from_pool =