From a18e659990a9cdd86643a819a943996769cb0195 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 10 Jun 2024 13:36:30 -0700 Subject: [PATCH 01/15] Qorb integration as connection pool for database --- Cargo.lock | 260 +++++++++++++++--- Cargo.toml | 7 +- dev-tools/omdb/src/bin/omdb/db.rs | 13 +- nexus-config/src/postgres_config.rs | 8 + nexus/db-queries/Cargo.toml | 2 + nexus/db-queries/src/db/collection_attach.rs | 28 +- nexus/db-queries/src/db/collection_detach.rs | 20 +- .../src/db/collection_detach_many.rs | 24 +- nexus/db-queries/src/db/collection_insert.rs | 12 +- .../src/db/datastore/db_metadata.rs | 13 +- .../db-queries/src/db/datastore/inventory.rs | 2 +- nexus/db-queries/src/db/datastore/mod.rs | 16 +- nexus/db-queries/src/db/datastore/probe.rs | 34 +-- .../src/db/datastore/pub_test_utils.rs | 2 +- nexus/db-queries/src/db/explain.rs | 11 +- nexus/db-queries/src/db/pagination.rs | 12 +- nexus/db-queries/src/db/pool.rs | 176 ++++++------ nexus/db-queries/src/db/pool_connection.rs | 65 ++--- .../db-queries/src/db/queries/external_ip.rs | 4 +- nexus/db-queries/src/db/queries/next_item.rs | 14 +- .../src/db/queries/region_allocation.rs | 4 +- .../virtual_provisioning_collection_update.rs | 16 +- nexus/db-queries/src/db/queries/vpc_subnet.rs | 4 +- nexus/db-queries/src/db/saga_recovery.rs | 3 +- nexus/src/bin/schema-updater.rs | 3 +- nexus/src/context.rs | 81 ++---- nexus/src/populate.rs | 17 +- nexus/tests/integration_tests/schema.rs | 8 +- 28 files changed, 522 insertions(+), 337 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b684da1dda..ccb98d1b8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -255,12 +255,14 @@ checksum = "9b34d609dfbaf33d6889b2b7106d3ca345eacad44200913df5ba02bfd31d2ba9" [[package]] name = "async-bb8-diesel" -version = "0.1.0" -source = "git+https://github.com/oxidecomputer/async-bb8-diesel?rev=ed7ab5ef0513ba303d33efd41d3e9e381169d59b#ed7ab5ef0513ba303d33efd41d3e9e381169d59b" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ebc03a2806f66f36513d65e0a7f34200382230250cadcf8a8397cfbe3f26b795" dependencies = [ "async-trait", "bb8", "diesel", + "futures", "thiserror", "tokio", ] @@ -1258,7 +1260,7 @@ name = "crdb-seed" version = "0.1.0" dependencies = [ "anyhow", - "dropshot", + "dropshot 0.10.2-dev", "omicron-test-utils", "omicron-workspace-hack", "slog", @@ -1906,7 +1908,7 @@ dependencies = [ "chrono", "clap", "dns-service-client", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "http 0.2.12", "omicron-test-utils", @@ -2002,6 +2004,52 @@ dependencies = [ "uuid", ] +[[package]] +name = "dropshot" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a391eeedf8a75a188eb670327c704b7ab10eb2bb890e2ec0880dd21d609fb6e8" +dependencies = [ + "async-stream", + "async-trait", + "base64 0.22.1", + "bytes", + "camino", + "chrono", + "debug-ignore", + "dropshot_endpoint 0.10.1", + "form_urlencoded", + "futures", + "hostname 0.4.0", + "http 0.2.12", + "hyper 0.14.28", + "indexmap 2.2.6", + "multer", + "openapiv3", + "paste", + "percent-encoding", + "rustls 0.22.4", + "rustls-pemfile 2.1.2", + "schemars", + "scopeguard", + "serde", + "serde_json", + "serde_path_to_error", + "serde_urlencoded", + "sha1", + "slog", + "slog-async", + "slog-bunyan", + "slog-json", + "slog-term", + "tokio", + "tokio-rustls 0.25.0", + "toml 0.8.13", + "uuid", + "version_check", + "waitgroup", +] + [[package]] name = "dropshot" version = "0.10.2-dev" @@ -2014,7 +2062,7 @@ dependencies = [ "camino", "chrono", "debug-ignore", - "dropshot_endpoint", + "dropshot_endpoint 0.10.2-dev", "form_urlencoded", "futures", "hostname 0.4.0", @@ -2048,6 +2096,19 @@ dependencies = [ "waitgroup", ] +[[package]] +name = "dropshot_endpoint" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9058c9c7e4a6b378cd12e71dc155bb15d0d4f8e1e6039ce2cf0a7c0c81043e33" +dependencies = [ + "proc-macro2", + "quote", + "serde", + "serde_tokenstream", + "syn 2.0.64", +] + [[package]] name = "dropshot_endpoint" version = "0.10.2-dev" @@ -2235,6 +2296,18 @@ dependencies = [ "syn 1.0.109", ] +[[package]] +name = "enum-as-inner" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ffccbb6966c05b32ef8fbac435df276c4ae4d3dc55a8cd0eb9745e6c12f546a" +dependencies = [ + "heck 0.4.1", + "proc-macro2", + "quote", + "syn 2.0.64", +] + [[package]] name = "env_logger" version = "0.9.3" @@ -2720,7 +2793,7 @@ name = "gateway-test-utils" version = "0.1.0" dependencies = [ "camino", - "dropshot", + "dropshot 0.10.2-dev", "gateway-messages", "omicron-gateway", "omicron-test-utils", @@ -3045,6 +3118,51 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6fe2267d4ed49bc07b63801559be28c718ea06c4738b7a03c94df7386d2cde46" +[[package]] +name = "hickory-proto" +version = "0.24.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "07698b8420e2f0d6447a436ba999ec85d8fbf2a398bbd737b82cac4a2e96e512" +dependencies = [ + "async-trait", + "cfg-if", + "data-encoding", + "enum-as-inner 0.6.0", + "futures-channel", + "futures-io", + "futures-util", + "idna 0.4.0", + "ipnet", + "once_cell", + "rand 0.8.5", + "thiserror", + "tinyvec", + "tokio", + "tracing", + "url", +] + +[[package]] +name = "hickory-resolver" +version = "0.24.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "28757f23aa75c98f254cf0405e6d8c25b831b32921b050a66692427679b1f243" +dependencies = [ + "cfg-if", + "futures-util", + "hickory-proto", + "ipconfig", + "lru-cache", + "once_cell", + "parking_lot 0.12.2", + "rand 0.8.5", + "resolv-conf", + "smallvec 1.13.2", + "thiserror", + "tokio", + "tracing", +] + [[package]] name = "highway" version = "1.1.0" @@ -3427,6 +3545,16 @@ dependencies = [ "unicode-normalization", ] +[[package]] +name = "idna" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d20d6b07bfbc108882d88ed8e37d39636dcc260e15e30c45e6ba089610b917c" +dependencies = [ + "unicode-bidi", + "unicode-normalization", +] + [[package]] name = "idna" version = "0.5.0" @@ -3631,7 +3759,7 @@ dependencies = [ "anyhow", "async-trait", "clap", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "hyper 0.14.28", "installinator-common", @@ -3686,7 +3814,7 @@ dependencies = [ "chrono", "dns-server", "dns-service-client", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "futures", "hyper 0.14.28", @@ -3713,7 +3841,7 @@ version = "0.1.0" dependencies = [ "anyhow", "clap", - "dropshot", + "dropshot 0.10.2-dev", "internal-dns", "omicron-common", "omicron-workspace-hack", @@ -4463,7 +4591,7 @@ dependencies = [ "base64 0.22.1", "chrono", "cookie 0.18.1", - "dropshot", + "dropshot 0.10.2-dev", "futures", "headers", "http 0.2.12", @@ -4519,7 +4647,7 @@ version = "0.1.0" dependencies = [ "anyhow", "camino", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "libc", "omicron-common", @@ -4609,7 +4737,7 @@ dependencies = [ "db-macros", "diesel", "diesel-dtrace", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "futures", "gateway-client", @@ -4645,6 +4773,7 @@ dependencies = [ "pq-sys", "predicates", "pretty_assertions", + "qorb", "rand 0.8.5", "rcgen", "ref-cast", @@ -4876,7 +5005,7 @@ dependencies = [ "crucible-agent-client", "dns-server", "dns-service-client", - "dropshot", + "dropshot 0.10.2-dev", "futures", "gateway-messages", "gateway-test-utils", @@ -5246,7 +5375,7 @@ dependencies = [ "chrono", "clap", "csv", - "dropshot", + "dropshot 0.10.2-dev", "http 0.2.12", "illumos-utils", "nexus-test-utils", @@ -5278,7 +5407,7 @@ dependencies = [ "camino", "camino-tempfile", "chrono", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "futures", "hex", @@ -5348,7 +5477,7 @@ dependencies = [ "camino", "camino-tempfile", "clap", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "futures", "gateway-messages", @@ -5382,7 +5511,7 @@ dependencies = [ "base64 0.22.1", "camino", "clap", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "futures", "gateway-messages", @@ -5441,7 +5570,7 @@ dependencies = [ "dns-server", "dns-service-client", "dpd-client", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "fatfs", "futures", @@ -5559,7 +5688,7 @@ dependencies = [ "crucible-agent-client", "csv", "diesel", - "dropshot", + "dropshot 0.10.2-dev", "dyn-clone", "expectorate", "futures", @@ -5714,7 +5843,7 @@ dependencies = [ "dns-server", "dns-service-client", "dpd-client", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "flate2", "flume", @@ -5798,7 +5927,7 @@ dependencies = [ "atomicwrites", "camino", "camino-tempfile", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "filetime", "gethostname", @@ -6235,7 +6364,7 @@ dependencies = [ "camino", "chrono", "clap", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "futures", "hyper 0.14.28", @@ -6280,7 +6409,7 @@ dependencies = [ "chrono", "clap", "crossterm", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "futures", "highway", @@ -6320,7 +6449,7 @@ version = "0.1.0" dependencies = [ "cfg-if", "chrono", - "dropshot", + "dropshot 0.10.2-dev", "futures", "http 0.2.12", "kstat-rs", @@ -6352,7 +6481,7 @@ dependencies = [ "anyhow", "chrono", "clap", - "dropshot", + "dropshot 0.10.2-dev", "internal-dns", "nexus-client", "omicron-common", @@ -7231,7 +7360,7 @@ dependencies = [ "atty", "base64 0.21.7", "clap", - "dropshot", + "dropshot 0.10.2-dev", "futures", "hyper 0.14.28", "progenitor", @@ -7318,6 +7447,32 @@ dependencies = [ "psl-types", ] +[[package]] +name = "qorb" +version = "0.0.1" +source = "git+https://github.com/oxidecomputer/qorb?branch=master#64d959b94b594499e28df3117dfdeafad3ab9036" +dependencies = [ + "anyhow", + "async-bb8-diesel", + "async-trait", + "debug-ignore", + "derive-where", + "diesel", + "diesel-dtrace", + "dropshot 0.10.1", + "futures", + "hickory-resolver", + "rand 0.8.5", + "schemars", + "serde", + "serde_json", + "thiserror", + "tokio", + "tokio-stream", + "tokio-tungstenite 0.23.0", + "tracing", +] + [[package]] name = "quick-error" version = "1.2.3" @@ -7521,7 +7676,7 @@ dependencies = [ "camino-tempfile", "clap", "dns-service-client", - "dropshot", + "dropshot 0.10.2-dev", "expectorate", "humantime", "indexmap 2.2.6", @@ -9068,7 +9223,7 @@ dependencies = [ "anyhow", "async-trait", "clap", - "dropshot", + "dropshot 0.10.2-dev", "futures", "gateway-messages", "hex", @@ -9780,9 +9935,9 @@ dependencies = [ [[package]] name = "tokio" -version = "1.37.0" +version = "1.38.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1adbebffeca75fcfd058afa480fb6c0b81e165a0323f9c9d39c9697e37c46787" +checksum = "ba4f4a02a7a80d6f274636f0aa95c7e383b912d41fe721a31f29e29698585a4a" dependencies = [ "backtrace", "bytes", @@ -9799,9 +9954,9 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.2.0" +version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5b8a1e28f2deaa14e508979454cb3a223b10b938b45af148bc0986de36f1923b" +checksum = "5f5ae998a069d4b5aba8ee9dad856af7d520c3699e6159b185c2acd48155d39a" dependencies = [ "proc-macro2", "quote", @@ -9874,6 +10029,7 @@ dependencies = [ "futures-core", "pin-project-lite", "tokio", + "tokio-util", ] [[package]] @@ -9900,6 +10056,18 @@ dependencies = [ "tungstenite 0.21.0", ] +[[package]] +name = "tokio-tungstenite" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "becd34a233e7e31a3dbf7c7241b38320f57393dcae8e7324b0167d21b8e320b0" +dependencies = [ + "futures-util", + "log", + "tokio", + "tungstenite 0.23.0", +] + [[package]] name = "tokio-util" version = "0.7.11" @@ -10123,7 +10291,7 @@ dependencies = [ "async-trait", "cfg-if", "data-encoding", - "enum-as-inner", + "enum-as-inner 0.5.1", "futures-channel", "futures-io", "futures-util", @@ -10168,7 +10336,7 @@ dependencies = [ "async-trait", "bytes", "cfg-if", - "enum-as-inner", + "enum-as-inner 0.5.1", "futures-executor", "futures-util", "serde", @@ -10312,6 +10480,24 @@ dependencies = [ "utf-8", ] +[[package]] +name = "tungstenite" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6e2e2ce1e47ed2994fd43b04c8f618008d4cabdd5ee34027cf14f9d918edd9c8" +dependencies = [ + "byteorder", + "bytes", + "data-encoding", + "http 1.1.0", + "httparse", + "log", + "rand 0.8.5", + "sha1", + "thiserror", + "utf-8", +] + [[package]] name = "typed-path" version = "0.7.1" @@ -10492,7 +10678,7 @@ dependencies = [ "clap", "debug-ignore", "display-error-chain", - "dropshot", + "dropshot 0.10.2-dev", "futures", "hex", "hubtools", @@ -11001,7 +11187,7 @@ dependencies = [ "debug-ignore", "display-error-chain", "dpd-client", - "dropshot", + "dropshot 0.10.2-dev", "either", "expectorate", "flate2", @@ -11485,7 +11671,7 @@ version = "0.1.0" dependencies = [ "anyhow", "clap", - "dropshot", + "dropshot 0.10.2-dev", "illumos-utils", "omicron-common", "omicron-workspace-hack", diff --git a/Cargo.toml b/Cargo.toml index 489c7a1552..1cddb6d84a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -217,7 +217,7 @@ api_identity = { path = "api_identity" } approx = "0.5.1" assert_matches = "1.5.0" assert_cmd = "2.0.14" -async-bb8-diesel = { git = "https://github.com/oxidecomputer/async-bb8-diesel", rev = "ed7ab5ef0513ba303d33efd41d3e9e381169d59b" } +async-bb8-diesel = "0.2" async-trait = "0.1.80" atomicwrites = "0.4.3" authz-macros = { path = "nexus/authz-macros" } @@ -395,6 +395,9 @@ bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a0 propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a033babc054db9eff5b59dee978d2b0d76" } propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a033babc054db9eff5b59dee978d2b0d76" } proptest = "1.4.0" +# TODO: Patch me before merging! +# qorb = { path = "../../qorb" } +qorb = { git = "https://github.com/oxidecomputer/qorb", branch = "master" } quote = "1.0" rand = "0.8.5" rand_core = "0.6.4" @@ -472,7 +475,7 @@ textwrap = "0.16.1" test-strategy = "0.3.1" thiserror = "1.0" tofino = { git = "http://github.com/oxidecomputer/tofino", branch = "main" } -tokio = "1.37.0" +tokio = "1.38.0" tokio-postgres = { version = "0.7", features = [ "with-chrono-0_4", "with-uuid-1" ] } tokio-stream = "0.1.15" tokio-tungstenite = "0.20" diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index be4e1e8696..bac93d057f 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(&log.clone(), &db_config)); + let pool = + Arc::new(db::Pool::new_qorb_single_host_blocking(&db_config).await); // 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()` @@ -3069,7 +3070,7 @@ async fn cmd_db_inventory( } async fn cmd_db_inventory_baseboard_ids( - conn: &DataStoreConnection<'_>, + conn: &DataStoreConnection, limit: NonZeroU32, ) -> Result<(), anyhow::Error> { #[derive(Tabled)] @@ -3106,7 +3107,7 @@ async fn cmd_db_inventory_baseboard_ids( } async fn cmd_db_inventory_cabooses( - conn: &DataStoreConnection<'_>, + conn: &DataStoreConnection, limit: NonZeroU32, ) -> Result<(), anyhow::Error> { #[derive(Tabled)] @@ -3147,7 +3148,7 @@ async fn cmd_db_inventory_cabooses( } async fn cmd_db_inventory_physical_disks( - conn: &DataStoreConnection<'_>, + conn: &DataStoreConnection, limit: NonZeroU32, args: PhysicalDisksArgs, ) -> Result<(), anyhow::Error> { @@ -3204,7 +3205,7 @@ async fn cmd_db_inventory_physical_disks( } async fn cmd_db_inventory_rot_pages( - conn: &DataStoreConnection<'_>, + conn: &DataStoreConnection, limit: NonZeroU32, ) -> Result<(), anyhow::Error> { #[derive(Tabled)] @@ -3239,7 +3240,7 @@ async fn cmd_db_inventory_rot_pages( } async fn cmd_db_inventory_collections_list( - conn: &DataStoreConnection<'_>, + conn: &DataStoreConnection, limit: NonZeroU32, ) -> Result<(), anyhow::Error> { #[derive(Tabled)] diff --git a/nexus-config/src/postgres_config.rs b/nexus-config/src/postgres_config.rs index 2509ae4fca..fc49f4df01 100644 --- a/nexus-config/src/postgres_config.rs +++ b/nexus-config/src/postgres_config.rs @@ -5,6 +5,7 @@ //! Common objects used for configuration use std::fmt; +use std::net::SocketAddr; use std::ops::Deref; use std::str::FromStr; @@ -32,6 +33,13 @@ impl PostgresConfigWithUrl { pub fn url(&self) -> String { self.url_raw.clone() } + + /// Accesses the first ip / port pair within the URL. + pub fn address(&self) -> SocketAddr { + let ip = self.config.get_hostaddrs()[0]; + let port = self.config.get_ports()[0]; + SocketAddr::new(ip, port) + } } impl FromStr for PostgresConfigWithUrl { diff --git a/nexus/db-queries/Cargo.toml b/nexus/db-queries/Cargo.toml index cb7061f4ce..3d6a2ddfc4 100644 --- a/nexus/db-queries/Cargo.toml +++ b/nexus/db-queries/Cargo.toml @@ -22,6 +22,7 @@ diesel.workspace = true diesel-dtrace.workspace = true dropshot.workspace = true futures.workspace = true +internal-dns.workspace = true ipnetwork.workspace = true macaddr.workspace = true once_cell.workspace = true @@ -29,6 +30,7 @@ oxnet.workspace = true paste.workspace = true # See omicron-rpaths for more about the "pq-sys" dependency. pq-sys = "*" +qorb = { workspace = true, features = [ "diesel_pg", "qtop" ] } rand.workspace = true ref-cast.workspace = true schemars.workspace = true diff --git a/nexus/db-queries/src/db/collection_attach.rs b/nexus/db-queries/src/db/collection_attach.rs index fccc1aa324..08bb354fe2 100644 --- a/nexus/db-queries/src/db/collection_attach.rs +++ b/nexus/db-queries/src/db/collection_attach.rs @@ -564,9 +564,7 @@ where mod test { use super::*; use crate::db::{self, identity::Resource as IdentityResource}; - use async_bb8_diesel::{ - AsyncRunQueryDsl, AsyncSimpleConnection, ConnectionManager, - }; + use async_bb8_diesel::{AsyncRunQueryDsl, AsyncSimpleConnection}; use chrono::Utc; use db_macros::Resource; use diesel::expression_methods::ExpressionMethods; @@ -603,8 +601,8 @@ mod test { async fn setup_db( pool: &crate::db::Pool, - ) -> bb8::PooledConnection> { - let connection = pool.pool().get().await.unwrap(); + ) -> crate::db::datastore::DataStoreConnection { + let connection = pool.claim().await.unwrap(); (*connection) .batch_execute_async( "CREATE SCHEMA IF NOT EXISTS test_schema; \ @@ -859,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -888,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -925,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -973,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -1022,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -1078,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -1142,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -1249,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -1304,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -1349,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; 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 03e09d41ca..749adc2429 100644 --- a/nexus/db-queries/src/db/collection_detach.rs +++ b/nexus/db-queries/src/db/collection_detach.rs @@ -482,9 +482,7 @@ mod test { use super::*; use crate::db::collection_attach::DatastoreAttachTarget; use crate::db::{self, identity::Resource as IdentityResource}; - use async_bb8_diesel::{ - AsyncRunQueryDsl, AsyncSimpleConnection, ConnectionManager, - }; + use async_bb8_diesel::{AsyncRunQueryDsl, AsyncSimpleConnection}; use chrono::Utc; use db_macros::Resource; use diesel::expression_methods::ExpressionMethods; @@ -521,8 +519,8 @@ mod test { async fn setup_db( pool: &crate::db::Pool, - ) -> bb8::PooledConnection> { - let connection = pool.pool().get().await.unwrap(); + ) -> crate::db::datastore::DataStoreConnection { + let connection = pool.claim().await.unwrap(); (*connection) .batch_execute_async( "CREATE SCHEMA IF NOT EXISTS test_schema; \ @@ -786,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -814,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -850,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -890,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -954,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -998,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; 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 986cfb70b7..3aa6afa9b9 100644 --- a/nexus/db-queries/src/db/collection_detach_many.rs +++ b/nexus/db-queries/src/db/collection_detach_many.rs @@ -480,9 +480,7 @@ mod test { use super::*; use crate::db::collection_attach::DatastoreAttachTarget; use crate::db::{self, identity::Resource as IdentityResource}; - use async_bb8_diesel::{ - AsyncRunQueryDsl, AsyncSimpleConnection, ConnectionManager, - }; + use async_bb8_diesel::{AsyncRunQueryDsl, AsyncSimpleConnection}; use chrono::Utc; use db_macros::Resource; use diesel::expression_methods::ExpressionMethods; @@ -519,8 +517,8 @@ mod test { async fn setup_db( pool: &crate::db::Pool, - ) -> bb8::PooledConnection> { - let connection = pool.pool().get().await.unwrap(); + ) -> crate::db::datastore::DataStoreConnection { + let connection = pool.claim().await.unwrap(); (*connection) .batch_execute_async( "CREATE SCHEMA IF NOT EXISTS test_schema; \ @@ -778,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -808,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -849,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -892,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -937,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -993,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -1044,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -1102,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; 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 69906e6498..d65494b043 100644 --- a/nexus/db-queries/src/db/collection_insert.rs +++ b/nexus/db-queries/src/db/collection_insert.rs @@ -406,9 +406,7 @@ where mod test { use super::*; use crate::db::{self, identity::Resource as IdentityResource}; - use async_bb8_diesel::{ - AsyncRunQueryDsl, AsyncSimpleConnection, ConnectionManager, - }; + use async_bb8_diesel::{AsyncRunQueryDsl, AsyncSimpleConnection}; use chrono::{DateTime, Utc}; use db_macros::Resource; use diesel::expression_methods::ExpressionMethods; @@ -443,8 +441,8 @@ mod test { async fn setup_db( pool: &crate::db::Pool, - ) -> bb8::PooledConnection> { - let connection = pool.pool().get().await.unwrap(); + ) -> crate::db::datastore::DataStoreConnection { + let connection = pool.claim().await.unwrap(); (*connection) .batch_execute_async( "CREATE SCHEMA IF NOT EXISTS test_schema; \ @@ -560,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let conn = setup_db(&pool).await; @@ -590,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; 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 4169cc06bd..cc8ea6ce69 100644 --- a/nexus/db-queries/src/db/datastore/db_metadata.rs +++ b/nexus/db-queries/src/db/datastore/db_metadata.rs @@ -511,7 +511,8 @@ 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(&logctx.log, &cfg)); + let pool = + Arc::new(db::Pool::new_qorb_single_host_blocking(&cfg).await); let datastore = Arc::new(DataStore::new(&logctx.log, pool, None).await.unwrap()); @@ -559,8 +560,9 @@ 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(&logctx.log, &cfg)); - let conn = pool.pool().get().await.unwrap(); + let pool = + Arc::new(db::Pool::new_qorb_single_host_blocking(&cfg).await); + let conn = pool.claim().await.unwrap(); // Mimic the layout of "schema/crdb". let config_dir = Utf8TempDir::new().unwrap(); @@ -671,8 +673,9 @@ 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(&logctx.log, &cfg)); - let conn = pool.pool().get().await.unwrap(); + let pool = + Arc::new(db::Pool::new_qorb_single_host_blocking(&cfg).await); + let conn = pool.claim().await.unwrap(); // Mimic the layout of "schema/crdb". let config_dir = Utf8TempDir::new().unwrap(); diff --git a/nexus/db-queries/src/db/datastore/inventory.rs b/nexus/db-queries/src/db/datastore/inventory.rs index 289e443213..3a21455848 100644 --- a/nexus/db-queries/src/db/datastore/inventory.rs +++ b/nexus/db-queries/src/db/datastore/inventory.rs @@ -2166,7 +2166,7 @@ mod test { } impl CollectionCounts { - async fn new(conn: &DataStoreConnection<'_>) -> anyhow::Result { + async fn new(conn: &DataStoreConnection) -> anyhow::Result { conn.transaction_async(|conn| async move { conn.batch_execute_async(ALLOW_FULL_TABLE_SCAN_SQL) .await diff --git a/nexus/db-queries/src/db/datastore/mod.rs b/nexus/db-queries/src/db/datastore/mod.rs index 9ec3575860..3469cb6c7b 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -27,7 +27,7 @@ use crate::db::{ error::{public_error_from_diesel, ErrorHandler}, }; use ::oximeter::types::ProducerRegistry; -use async_bb8_diesel::{AsyncRunQueryDsl, ConnectionManager}; +use async_bb8_diesel::AsyncRunQueryDsl; use diesel::pg::Pg; use diesel::prelude::*; use diesel::query_builder::{QueryFragment, QueryId}; @@ -164,8 +164,8 @@ impl RunnableQuery for T where { } -pub type DataStoreConnection<'a> = - bb8::PooledConnection<'a, ConnectionManager>; +pub type DataStoreConnection = + qorb::claim::Handle>; pub struct DataStore { log: Logger, @@ -269,8 +269,7 @@ impl DataStore { opctx: &OpContext, ) -> Result { opctx.authorize(authz::Action::Query, &authz::DATABASE).await?; - let pool = self.pool.pool(); - let connection = pool.get().await.map_err(|err| { + let connection = self.pool.claim().await.map_err(|err| { Error::unavail(&format!("Failed to access DB connection: {err}")) })?; Ok(connection) @@ -284,7 +283,7 @@ impl DataStore { pub(super) async fn pool_connection_unauthorized( &self, ) -> Result { - let connection = self.pool.pool().get().await.map_err(|err| { + let connection = self.pool.claim().await.map_err(|err| { Error::unavail(&format!("Failed to access DB connection: {err}")) })?; Ok(connection) @@ -1570,7 +1569,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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; let datastore = DataStore::new(&logctx.log, Arc::new(pool), None).await.unwrap(); let conn = datastore.pool_connection_for_tests().await.unwrap(); @@ -1619,7 +1618,8 @@ 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(&logctx.log, &cfg)); + let pool = + Arc::new(db::Pool::new_qorb_single_host_blocking(&cfg).await); 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/probe.rs b/nexus/db-queries/src/db/datastore/probe.rs index a96f857163..6974754314 100644 --- a/nexus/db-queries/src/db/datastore/probe.rs +++ b/nexus/db-queries/src/db/datastore/probe.rs @@ -91,7 +91,7 @@ impl super::DataStore { use db::schema::probe::dsl; use db::schema::vpc_subnet::dsl as vpc_subnet_dsl; - let pool = self.pool_connection_authorized(opctx).await?; + let conn = self.pool_connection_authorized(opctx).await?; let probes = match pagparams { PaginatedBy::Id(pagparams) => { @@ -106,7 +106,7 @@ impl super::DataStore { .filter(dsl::project_id.eq(authz_project.id())) .filter(dsl::time_deleted.is_null()) .select(Probe::as_select()) - .load_async(&*pool) + .load_async(&*conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; @@ -128,7 +128,7 @@ impl super::DataStore { let db_subnet = vpc_subnet_dsl::vpc_subnet .filter(vpc_subnet_dsl::id.eq(interface.subnet_id)) .select(VpcSubnet::as_select()) - .first_async(&*pool) + .first_async(&*conn) .await .map_err(|e| { public_error_from_diesel(e, ErrorHandler::Server) @@ -155,7 +155,7 @@ impl super::DataStore { &self, opctx: &OpContext, probe: &Probe, - pool: &DataStoreConnection<'_>, + conn: &DataStoreConnection, ) -> LookupResult { use db::schema::vpc_subnet::dsl as vpc_subnet_dsl; @@ -172,7 +172,7 @@ impl super::DataStore { let db_subnet = vpc_subnet_dsl::vpc_subnet .filter(vpc_subnet_dsl::id.eq(interface.subnet_id)) .select(VpcSubnet::as_select()) - .first_async(&**pool) + .first_async(&**conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; @@ -201,20 +201,20 @@ impl super::DataStore { ) -> ListResultVec { use db::schema::probe::dsl; - let pool = self.pool_connection_authorized(opctx).await?; + let conn = self.pool_connection_authorized(opctx).await?; let probes = paginated(dsl::probe, dsl::id, pagparams) .filter(dsl::time_deleted.is_null()) .filter(dsl::sled.eq(sled)) .select(Probe::as_select()) - .load_async(&*pool) + .load_async(&*conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; let mut result = Vec::with_capacity(probes.len()); for probe in probes.into_iter() { - result.push(self.resolve_probe_info(opctx, &probe, &pool).await?); + result.push(self.resolve_probe_info(opctx, &probe, &conn).await?); } Ok(result) @@ -229,7 +229,7 @@ impl super::DataStore { ) -> LookupResult { use db::schema::probe; use db::schema::probe::dsl; - let pool = self.pool_connection_authorized(opctx).await?; + let conn = self.pool_connection_authorized(opctx).await?; let name_or_id = name_or_id.clone(); @@ -240,7 +240,7 @@ impl super::DataStore { .filter(probe::project_id.eq(authz_project.id())) .select(Probe::as_select()) .limit(1) - .first_async::(&*pool) + .first_async::(&*conn) .await .map_err(|e| { public_error_from_diesel( @@ -256,7 +256,7 @@ impl super::DataStore { .filter(probe::project_id.eq(authz_project.id())) .select(Probe::as_select()) .limit(1) - .first_async::(&*pool) + .first_async::(&*conn) .await .map_err(|e| { public_error_from_diesel( @@ -269,7 +269,7 @@ impl super::DataStore { }), }?; - self.resolve_probe_info(opctx, &probe, &pool).await + self.resolve_probe_info(opctx, &probe, &conn).await } /// Add a probe to the data store. @@ -282,7 +282,7 @@ impl super::DataStore { ) -> CreateResult { //TODO in transaction use db::schema::probe::dsl; - let pool = self.pool_connection_authorized(opctx).await?; + let conn = self.pool_connection_authorized(opctx).await?; let _eip = self .allocate_probe_ephemeral_ip( @@ -335,7 +335,7 @@ impl super::DataStore { let result = diesel::insert_into(dsl::probe) .values(probe.clone()) .returning(Probe::as_returning()) - .get_result_async(&*pool) + .get_result_async(&*conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; @@ -351,7 +351,7 @@ impl super::DataStore { ) -> DeleteResult { use db::schema::probe; use db::schema::probe::dsl; - let pool = self.pool_connection_authorized(opctx).await?; + let conn = self.pool_connection_authorized(opctx).await?; let name_or_id = name_or_id.clone(); @@ -363,7 +363,7 @@ impl super::DataStore { .filter(probe::project_id.eq(authz_project.id())) .select(probe::id) .limit(1) - .first_async::(&*pool) + .first_async::(&*conn) .await .map_err(|e| { public_error_from_diesel(e, ErrorHandler::Server) @@ -379,7 +379,7 @@ impl super::DataStore { .filter(dsl::id.eq(id)) .filter(dsl::project_id.eq(authz_project.id())) .set(dsl::time_deleted.eq(Utc::now())) - .execute_async(&*pool) + .execute_async(&*conn) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; 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 93a172bd15..f43d49980b 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(&logctx.log, &cfg)); + let pool = Arc::new(db::Pool::new_qorb_single_host_blocking(&cfg).await); 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 24fd993040..4104498eea 100644 --- a/nexus/db-queries/src/db/explain.rs +++ b/nexus/db-queries/src/db/explain.rs @@ -124,8 +124,7 @@ mod test { } async fn create_schema(pool: &db::Pool) { - pool.pool() - .get() + pool.claim() .await .unwrap() .batch_execute_async( @@ -145,8 +144,8 @@ 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(&logctx.log, &cfg); - let conn = pool.pool().get().await.unwrap(); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; + let conn = pool.claim().await.unwrap(); create_schema(&pool).await; @@ -170,8 +169,8 @@ 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(&logctx.log, &cfg); - let conn = pool.pool().get().await.unwrap(); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; + 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 4fc1cf5966..981f626baf 100644 --- a/nexus/db-queries/src/db/pagination.rs +++ b/nexus/db-queries/src/db/pagination.rs @@ -354,7 +354,7 @@ mod test { async fn populate_users(pool: &db::Pool, values: &Vec<(i64, i64)>) { use schema::test_users::dsl; - let conn = pool.pool().get().await.unwrap(); + let conn = pool.claim().await.unwrap(); // The indexes here work around the check that prevents full table // scans. @@ -392,7 +392,7 @@ mod test { pool: &db::Pool, query: BoxedQuery, ) -> Vec { - let conn = pool.pool().get().await.unwrap(); + let conn = pool.claim().await.unwrap(); query.select(User::as_select()).load_async(&*conn).await.unwrap() } @@ -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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; 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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; 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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; 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(&logctx.log, &cfg); + let pool = db::Pool::new_qorb_single_host_blocking(&cfg).await; use schema::test_users::dsl; diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index 497c8d97c5..ba50e789cc 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -3,108 +3,128 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. //! Database connection pooling -// This whole thing is a placeholder for prototyping. -// -// TODO-robustness TODO-resilience We will want to carefully think about the -// connection pool that we use and its parameters. It's not clear from the -// survey so far whether an existing module is suitable for our purposes. See -// the Cueball Internals document for details on the sorts of behaviors we'd -// like here. Even if by luck we stick with bb8, we definitely want to think -// through the various parameters. -// -// Notes about bb8's behavior: -// * When the database is completely offline, and somebody wants a connection, -// it still waits for the connection timeout before giving up. That seems -// like not what we want. (To be clear, this is a failure mode where we know -// the database is offline, not one where it's partitioned and we can't tell.) -// * Although the `build_unchecked()` builder allows the pool to start up with -// no connections established (good), it also _seems_ to not establish any -// connections even when it could, resulting in a latency bubble for the first -// operation after startup. That's not what we're looking for. -// // TODO-design Need TLS support (the types below hardcode NoTls). use super::Config as DbConfig; -use async_bb8_diesel::ConnectionError; -use async_bb8_diesel::ConnectionManager; +use qorb::backend; +use qorb::connectors::diesel_pg::DieselPgConnector; +use qorb::policy::Policy; +use qorb::resolver::{AllBackends, Resolver}; +use qorb::resolvers::dns::{DnsResolver, DnsResolverConfig}; +use qorb::service; +use std::collections::BTreeMap; +use std::net::SocketAddr; +use std::sync::Arc; +use tokio::sync::watch; pub use super::pool_connection::DbConnection; +type QorbConnection = async_bb8_diesel::Connection; +type QorbPool = qorb::pool::Pool; + /// Wrapper around a database connection pool. /// /// Expected to be used as the primary interface to the database. pub struct Pool { - pool: bb8::Pool>, + inner: QorbPool, } -impl Pool { - pub fn new(log: &slog::Logger, db_config: &DbConfig) -> Self { - // Make sure diesel-dtrace's USDT probes are enabled. - usdt::register_probes().expect("Failed to register USDT DTrace probes"); - Self::new_builder(log, db_config, bb8::Builder::new()) - } +// Provides an alternative to the DNS resolver for cases where we want to +// contact the pool directly. +struct SingleHostResolver { + tx: watch::Sender, +} - pub fn new_failfast_for_tests( - log: &slog::Logger, - db_config: &DbConfig, - ) -> Self { - Self::new_builder( - log, - db_config, - bb8::Builder::new() - .connection_timeout(std::time::Duration::from_millis(1)), - ) +impl SingleHostResolver { + fn new(config: &DbConfig) -> Self { + let backends = Arc::new(BTreeMap::from([( + backend::Name::new("singleton"), + backend::Backend { address: config.url.address() }, + )])); + let (tx, _rx) = watch::channel(backends.clone()); + Self { tx } } +} - fn new_builder( - log: &slog::Logger, - db_config: &DbConfig, - builder: bb8::Builder>, - ) -> Self { - let url = db_config.url.url(); - let log = log.new(o!( - "database_url" => url.clone(), - "component" => "db::Pool" - )); - info!(&log, "database connection pool"); - let error_sink = LoggingErrorSink::new(log); - let manager = ConnectionManager::::new(url); - let pool = builder - .connection_customizer(Box::new( - super::pool_connection::ConnectionCustomizer::new(), - )) - .error_sink(Box::new(error_sink)) - .build_unchecked(manager); - Pool { pool } +impl Resolver for SingleHostResolver { + fn monitor(&mut self) -> watch::Receiver { + self.tx.subscribe() } +} - /// Returns a reference to the underlying pool. - pub fn pool(&self) -> &bb8::Pool> { - &self.pool - } +fn make_dns_resolver( + bootstrap_dns: Vec, +) -> qorb::resolver::BoxedResolver { + Box::new(DnsResolver::new( + service::Name(internal_dns::ServiceName::Cockroach.srv_name()), + bootstrap_dns, + DnsResolverConfig::default(), + )) } -#[derive(Clone, Debug)] -struct LoggingErrorSink { - log: slog::Logger, +fn make_single_host_resolver( + config: &DbConfig, +) -> qorb::resolver::BoxedResolver { + Box::new(SingleHostResolver::new(config)) } -impl LoggingErrorSink { - fn new(log: slog::Logger) -> LoggingErrorSink { - LoggingErrorSink { log } - } +fn make_postgres_connector() -> qorb::backend::SharedConnector { + // Create postgres connections. + // + // TODO: "on acquire"? + // TODO: Connection timeout for failfast? + // + // We're currently relying on somewhat intrusive modifications to qorb to + // make these things possible. Might be worth a refactor? + let user = "root"; + let db = "omicron"; + let args = Some("sslmode=disable"); + Arc::new(DieselPgConnector::new(user, db, args)) } -impl bb8::ErrorSink for LoggingErrorSink { - fn sink(&self, error: ConnectionError) { - error!( - &self.log, - "database connection error"; - "error_message" => #%error - ); +impl Pool { + /// Creates a new qorb-backed connection pool to the database. + /// + /// Creating this pool does not necessarily wait for connections to become + /// available, as backends may shift over time. + /// + /// For tests that do want to await connections being available to claim, + /// see: [Self::new_qorb_single_host_blocking]. + pub fn new_qorb(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 policy = Policy::default(); + Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) } + } + + /// Creats a new qorb-backed connection pool to a single instance of the + /// database, and waits for that backend to come online. + /// + /// This is intended for tests that want assurance that the database backend + /// is online - in production, this assumption may not be valid, and + /// [Self::new_qorb] should be preferred. + pub async fn new_qorb_single_host_blocking(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 policy = Policy::default(); + let pool = + Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) }; + pool.inner.block_until_online().await; + pool } - fn boxed_clone(&self) -> Box> { - Box::new(self.clone()) + /// Returns a connection from the pool + pub async fn claim( + &self, + ) -> anyhow::Result> { + Ok(self.inner.claim().await?) } } diff --git a/nexus/db-queries/src/db/pool_connection.rs b/nexus/db-queries/src/db/pool_connection.rs index dae6a0ee51..c03cbb2afa 100644 --- a/nexus/db-queries/src/db/pool_connection.rs +++ b/nexus/db-queries/src/db/pool_connection.rs @@ -4,11 +4,6 @@ //! Customization that happens on each connection as they're acquired. -use async_bb8_diesel::AsyncSimpleConnection; -use async_bb8_diesel::Connection; -use async_bb8_diesel::ConnectionError; -use async_trait::async_trait; -use bb8::CustomizeConnection; use diesel::PgConnection; use diesel_dtrace::DTraceConnection; @@ -17,33 +12,33 @@ pub type DbConnection = DTraceConnection; pub const DISALLOW_FULL_TABLE_SCAN_SQL: &str = "set disallow_full_table_scans = on; set large_full_scan_rows = 0;"; -/// A customizer for all new connections made to CockroachDB, from Diesel. -#[derive(Debug)] -pub(crate) struct ConnectionCustomizer {} - -impl ConnectionCustomizer { - pub(crate) fn new() -> Self { - Self {} - } - - async fn disallow_full_table_scans( - &self, - conn: &mut Connection, - ) -> Result<(), ConnectionError> { - conn.batch_execute_async(DISALLOW_FULL_TABLE_SCAN_SQL).await?; - Ok(()) - } -} - -#[async_trait] -impl CustomizeConnection, ConnectionError> - for ConnectionCustomizer -{ - async fn on_acquire( - &self, - conn: &mut Connection, - ) -> Result<(), ConnectionError> { - self.disallow_full_table_scans(conn).await?; - Ok(()) - } -} +// /// A customizer for all new connections made to CockroachDB, from Diesel. +// #[derive(Debug)] +// pub(crate) struct ConnectionCustomizer {} +// +// impl ConnectionCustomizer { +// pub(crate) fn new() -> Self { +// Self {} +// } +// +// async fn disallow_full_table_scans( +// &self, +// conn: &mut Connection, +// ) -> Result<(), ConnectionError> { +// conn.batch_execute_async(DISALLOW_FULL_TABLE_SCAN_SQL).await?; +// Ok(()) +// } +// } +// +// #[async_trait] +// impl CustomizeConnection, ConnectionError> +// for ConnectionCustomizer +// { +// async fn on_acquire( +// &self, +// conn: &mut Connection, +// ) -> Result<(), ConnectionError> { +// self.disallow_full_table_scans(conn).await?; +// Ok(()) +// } +// } diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index 9bb6a44ea7..4aa9ac64ac 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -917,7 +917,9 @@ 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(&logctx.log, &cfg)); + let pool = Arc::new( + crate::db::Pool::new_qorb_single_host_blocking(&cfg).await, + ); 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 769c891349..4dc3a6a65d 100644 --- a/nexus/db-queries/src/db/queries/next_item.rs +++ b/nexus/db-queries/src/db/queries/next_item.rs @@ -616,7 +616,7 @@ mod tests { } async fn setup_test_schema(pool: &db::Pool) { - let connection = pool.pool().get().await.unwrap(); + let connection = pool.claim().await.unwrap(); (*connection) .batch_execute_async( "CREATE SCHEMA IF NOT EXISTS test_schema; \ @@ -708,8 +708,10 @@ 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(&logctx.log, &cfg)); - let conn = pool.pool().get().await.unwrap(); + let pool = Arc::new( + crate::db::Pool::new_qorb_single_host_blocking(&cfg).await, + ); + let conn = pool.claim().await.unwrap(); // We're going to operate on a separate table, for simplicity. setup_test_schema(&pool).await; @@ -770,8 +772,10 @@ 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(&logctx.log, &cfg)); - let conn = pool.pool().get().await.unwrap(); + let pool = Arc::new( + crate::db::Pool::new_qorb_single_host_blocking(&cfg).await, + ); + let conn = pool.claim().await.unwrap(); // We're going to operate on a separate table, for simplicity. setup_test_schema(&pool).await; diff --git a/nexus/db-queries/src/db/queries/region_allocation.rs b/nexus/db-queries/src/db/queries/region_allocation.rs index 83cc7483c9..f96612a4c6 100644 --- a/nexus/db-queries/src/db/queries/region_allocation.rs +++ b/nexus/db-queries/src/db/queries/region_allocation.rs @@ -427,8 +427,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 = crate::db::Pool::new(&logctx.log, &cfg); - let conn = pool.pool().get().await.unwrap(); + let pool = crate::db::Pool::new_qorb_single_host_blocking(&cfg).await; + let conn = pool.claim().await.unwrap(); let volume_id = Uuid::new_v4(); let block_size = 512; 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 b3c1a569b0..4af3fcb022 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,8 +590,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 = crate::db::Pool::new(&logctx.log, &cfg); - let conn = pool.pool().get().await.unwrap(); + let pool = crate::db::Pool::new_qorb_single_host_blocking(&cfg).await; + let conn = pool.claim().await.unwrap(); let id = Uuid::nil(); let project_id = Uuid::nil(); @@ -619,8 +619,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 = crate::db::Pool::new(&logctx.log, &cfg); - let conn = pool.pool().get().await.unwrap(); + let pool = crate::db::Pool::new_qorb_single_host_blocking(&cfg).await; + let conn = pool.claim().await.unwrap(); let id = Uuid::nil(); let project_id = Uuid::nil(); @@ -646,8 +646,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 = crate::db::Pool::new(&logctx.log, &cfg); - let conn = pool.pool().get().await.unwrap(); + let pool = crate::db::Pool::new_qorb_single_host_blocking(&cfg).await; + let conn = pool.claim().await.unwrap(); let id = Uuid::nil(); let project_id = Uuid::nil(); @@ -672,8 +672,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 = crate::db::Pool::new(&logctx.log, &cfg); - let conn = pool.pool().get().await.unwrap(); + let pool = crate::db::Pool::new_qorb_single_host_blocking(&cfg).await; + let conn = pool.claim().await.unwrap(); let id = Uuid::nil(); let max_instance_gen = 0; diff --git a/nexus/db-queries/src/db/queries/vpc_subnet.rs b/nexus/db-queries/src/db/queries/vpc_subnet.rs index 72f2771a1e..b0670c13c9 100644 --- a/nexus/db-queries/src/db/queries/vpc_subnet.rs +++ b/nexus/db-queries/src/db/queries/vpc_subnet.rs @@ -436,7 +436,9 @@ 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(&logctx.log, &cfg)); + let pool = Arc::new( + crate::db::Pool::new_qorb_single_host_blocking(&cfg).await, + ); 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 25f8ff788d..f33ebfe204 100644 --- a/nexus/db-queries/src/db/saga_recovery.rs +++ b/nexus/db-queries/src/db/saga_recovery.rs @@ -327,7 +327,8 @@ 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(log, &cfg)); + let pool = + Arc::new(db::Pool::new_qorb_single_host_blocking(&cfg).await); 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 7fe1ed84a4..655ccaecbf 100644 --- a/nexus/src/bin/schema-updater.rs +++ b/nexus/src/bin/schema-updater.rs @@ -71,7 +71,8 @@ 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(&log, &crdb_cfg)); + let pool = + Arc::new(db::Pool::new_qorb_single_host_blocking(&crdb_cfg).await); 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 1512671056..dec30832ca 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -11,9 +11,7 @@ use authn::external::token::HttpAuthnToken; use authn::external::HttpAuthnScheme; use camino::Utf8PathBuf; use chrono::Duration; -use internal_dns::ServiceName; use nexus_config::NexusConfig; -use nexus_config::PostgresConfigWithUrl; use nexus_config::SchemeName; use nexus_db_queries::authn::external::session_cookie::SessionStore; use nexus_db_queries::authn::ConsoleSessionWithSiloId; @@ -25,7 +23,6 @@ use oximeter::types::ProducerRegistry; use oximeter_instruments::http::{HttpService, LatencyTracker}; use slog::Logger; use std::env; -use std::str::FromStr; use std::sync::Arc; use uuid::Uuid; @@ -210,7 +207,7 @@ impl ServerContext { // nexus in dev for everyone // Set up DNS Client - let resolver = match config.deployment.internal_dns { + let (resolver, dns_addrs) = match config.deployment.internal_dns { nexus_config::InternalDns::FromSubnet { subnet } => { let az_subnet = Ipv6Subnet::::new(subnet.net().addr()); @@ -219,11 +216,21 @@ impl ServerContext { "Setting up resolver using DNS servers for subnet: {:?}", az_subnet ); - internal_dns::resolver::Resolver::new_from_subnet( - log.new(o!("component" => "DnsResolver")), - az_subnet, + let resolver = + internal_dns::resolver::Resolver::new_from_subnet( + log.new(o!("component" => "DnsResolver")), + az_subnet, + ) + .map_err(|e| { + format!("Failed to create DNS resolver: {}", e) + })?; + + ( + resolver, + internal_dns::resolver::Resolver::servers_from_subnet( + az_subnet, + ), ) - .map_err(|e| format!("Failed to create DNS resolver: {}", e))? } nexus_config::InternalDns::FromAddress { address } => { info!( @@ -231,56 +238,20 @@ impl ServerContext { "Setting up resolver using DNS address: {:?}", address ); - internal_dns::resolver::Resolver::new_from_addrs( - log.new(o!("component" => "DnsResolver")), - &[address], - ) - .map_err(|e| format!("Failed to create DNS resolver: {}", e))? - } - }; + let resolver = + internal_dns::resolver::Resolver::new_from_addrs( + log.new(o!("component" => "DnsResolver")), + &[address], + ) + .map_err(|e| { + format!("Failed to create DNS resolver: {}", e) + })?; - // Set up DB pool - let url = match &config.deployment.database { - nexus_config::Database::FromUrl { url } => url.clone(), - nexus_config::Database::FromDns => { - info!(log, "Accessing DB url from DNS"); - // It's been requested but unfortunately not supported to - // directly connect using SRV based lookup. - // TODO-robustness: the set of cockroachdb hosts we'll use will - // be fixed to whatever we got back from DNS at Nexus start. - // This means a new cockroachdb instance won't picked up until - // Nexus restarts. - let addrs = loop { - match resolver - .lookup_all_socket_v6(ServiceName::Cockroach) - .await - { - Ok(addrs) => break addrs, - Err(e) => { - warn!( - log, - "Failed to lookup cockroach addresses: {e}" - ); - tokio::time::sleep(std::time::Duration::from_secs( - 1, - )) - .await; - } - } - }; - let addrs_str = addrs - .iter() - .map(ToString::to_string) - .collect::>() - .join(","); - info!(log, "DB addresses: {}", addrs_str); - PostgresConfigWithUrl::from_str(&format!( - "postgresql://root@{addrs_str}/omicron?sslmode=disable", - )) - .map_err(|e| format!("Cannot parse Postgres URL: {}", e))? + (resolver, vec![address]) } }; - let pool = db::Pool::new(&log, &db::Config { url }); + + let pool = db::Pool::new_qorb(dns_addrs); let nexus = Nexus::new_with_id( rack_id, log.new(o!("component" => "nexus")), diff --git a/nexus/src/populate.rs b/nexus/src/populate.rs index 724b25162d..ad27ba8f89 100644 --- a/nexus/src/populate.rs +++ b/nexus/src/populate.rs @@ -380,7 +380,8 @@ 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(&logctx.log, &cfg)); + let pool = + Arc::new(db::Pool::new_qorb_single_host_blocking(&cfg).await); let datastore = Arc::new( db::DataStore::new(&logctx.log, pool, None).await.unwrap(), ); @@ -422,19 +423,13 @@ mod test { }) .unwrap(); - // Test again with the database offline. In principle we could do this - // immediately without creating a new pool and datastore. However, the - // pool's default behavior is to wait 30 seconds for a connection, which - // makes this test take a long time. (See the note in - // nexus/src/db/pool.rs about this.) So let's create a pool with an - // arbitrarily short timeout now. (We wouldn't want to do this above - // because we do want to wait a bit when we expect things to work, in - // case the test system is busy.) + // Test again with the database offline.In principle we could do this + // immediately without creating a new pool and datastore. // - // Anyway, if we try again with a broken database, we should get a + // 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_failfast_for_tests(&logctx.log, &cfg)); + Arc::new(db::Pool::new_qorb_single_host_blocking(&cfg).await); // 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 bf73855ea7..f740e28f3c 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -954,12 +954,12 @@ async fn dbinit_equals_sum_of_all_up() { // Create a connection pool after we apply the first schema version but // 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( - log, + let pool = nexus_db_queries::db::Pool::new_qorb_single_host_blocking( &nexus_db_queries::db::Config { url: crdb.pg_config().clone() }, - ); + ) + .await; let conn_from_pool = - pool.pool().get().await.expect("failed to get pooled connection"); + pool.claim().await.expect("failed to get pooled connection"); // Go from the second version to the latest version. for version in all_versions.iter_versions().skip(1) { From 4695244cbccba79561bfb1675811a46465184acf Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 10 Jun 2024 17:21:57 -0700 Subject: [PATCH 02/15] Getting there with tests --- nexus-config/src/postgres_config.rs | 15 ++++++++++++++- nexus/db-queries/src/db/pool.rs | 29 ++++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/nexus-config/src/postgres_config.rs b/nexus-config/src/postgres_config.rs index fc49f4df01..b2be7ff79f 100644 --- a/nexus-config/src/postgres_config.rs +++ b/nexus-config/src/postgres_config.rs @@ -35,8 +35,21 @@ impl PostgresConfigWithUrl { } /// Accesses the first ip / port pair within the URL. + /// + /// # Safety + /// + /// This method makes the assumption that the hostname has at least one + /// "host IP / port" pair which can be extracted. If the supplied URL + /// does not have such a pair, this function will panic. + // Yes, panicking in the above scenario sucks. But this type is already + // pretty ubiquitous within Omicron, and integration with the qorb + // connection pooling library requires access to database by SocketAddr. pub fn address(&self) -> SocketAddr { - let ip = self.config.get_hostaddrs()[0]; + let tokio_postgres::config::Host::Tcp(host) = &self.config.get_hosts()[0] else { + panic!("Non-TCP hostname"); + }; + let ip: std::net::IpAddr = host.parse().expect("Failed to parse host as IP address"); + let port = self.config.get_ports()[0]; SocketAddr::new(ip, port) } diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index ba50e789cc..0c7875431d 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -114,10 +114,37 @@ impl Pool { let resolver = make_single_host_resolver(db_config); let connector = make_postgres_connector(); - let policy = Policy::default(); + let spares_wanted = 10; + let policy = Policy { + spares_wanted, + max_slots: 20, + ..Default::default() + }; let pool = Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) }; + + // I wish this was enough to work for our tests, but many of them try to + // grab multiple connections back-to-back. Therefore, we can't "just" + // wait for the pool to come online, we need to wait until we have some + // confidence that enough connections exist to be used by our test. + // + // Rather than flaking, we wait until unclaimed slots are made + // accessible. + // + // Since this is intended to be used by tests connecting to a single + // host locally, we SHOULD be able to make these connections. pool.inner.block_until_online().await; + + let mut rx = pool.inner.stats().rx.clone(); + let backends = rx.wait_for(|value| !value.is_empty()) + .await + .expect("Database never became ready and pool was dropped"); + let stats = backends.values().next().unwrap(); + + while stats.get().unclaimed_slots < spares_wanted { + tokio::time::sleep(tokio::time::Duration::from_millis(10)).await; + } + pool } From 0b1e300cd42221dc426b0dfdb489fa3c874e39af Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 10 Jun 2024 17:54:17 -0700 Subject: [PATCH 03/15] use a ton of spares --- nexus-config/src/postgres_config.rs | 7 +++++-- nexus/db-queries/src/db/pool.rs | 20 ++++++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/nexus-config/src/postgres_config.rs b/nexus-config/src/postgres_config.rs index b2be7ff79f..6f60c8b4fb 100644 --- a/nexus-config/src/postgres_config.rs +++ b/nexus-config/src/postgres_config.rs @@ -45,10 +45,13 @@ impl PostgresConfigWithUrl { // pretty ubiquitous within Omicron, and integration with the qorb // connection pooling library requires access to database by SocketAddr. pub fn address(&self) -> SocketAddr { - let tokio_postgres::config::Host::Tcp(host) = &self.config.get_hosts()[0] else { + let tokio_postgres::config::Host::Tcp(host) = + &self.config.get_hosts()[0] + else { panic!("Non-TCP hostname"); }; - let ip: std::net::IpAddr = host.parse().expect("Failed to parse host as IP address"); + let ip: std::net::IpAddr = + host.parse().expect("Failed to parse host as IP address"); let port = self.config.get_ports()[0]; SocketAddr::new(ip, port) diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index 0c7875431d..7ff9660618 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -8,7 +8,7 @@ use super::Config as DbConfig; use qorb::backend; use qorb::connectors::diesel_pg::DieselPgConnector; -use qorb::policy::Policy; +use qorb::policy::{Policy, SetConfig}; use qorb::resolver::{AllBackends, Resolver}; use qorb::resolvers::dns::{DnsResolver, DnsResolverConfig}; use qorb::service; @@ -114,10 +114,21 @@ impl Pool { let resolver = make_single_host_resolver(db_config); let connector = make_postgres_connector(); - let spares_wanted = 10; + // NOTE: this seems like overkill, but it's actually needed + // for "test_iam_roles_behavior" -- even 50 spares doesn't work + // reliably. + // + // Unclear to me who should eat the burden for this. Should it be + // configurable? Should we let tests "await" a little longer during + // connection claiming? This wait-on-claim behavior is what bb8 did, + // but we explicitly wanted to avoid that in cases where the databases + // are offline. + let spares_wanted = 100; + let max = spares_wanted * 2; let policy = Policy { spares_wanted, - max_slots: 20, + max_slots: max, + set_config: SetConfig { max_count: max, ..Default::default() }, ..Default::default() }; let pool = @@ -136,7 +147,8 @@ impl Pool { pool.inner.block_until_online().await; let mut rx = pool.inner.stats().rx.clone(); - let backends = rx.wait_for(|value| !value.is_empty()) + let backends = rx + .wait_for(|value| !value.is_empty()) .await .expect("Database never became ready and pool was dropped"); let stats = backends.values().next().unwrap(); From 91375c42446bd14c2304231c62eaa486556ff7c0 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 11 Jun 2024 16:30:49 -0700 Subject: [PATCH 04/15] Continue patching tests --- 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 | 9 +-- nexus/db-queries/src/db/datastore/mod.rs | 5 +- .../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 | 77 +++++++------------ .../db-queries/src/db/queries/external_ip.rs | 5 +- nexus/db-queries/src/db/queries/next_item.rs | 8 +- .../src/db/queries/region_allocation.rs | 2 +- .../virtual_provisioning_collection_update.rs | 8 +- nexus/db-queries/src/db/queries/vpc_subnet.rs | 4 +- nexus/db-queries/src/db/saga_recovery.rs | 3 +- nexus/src/bin/schema-updater.rs | 3 +- nexus/src/context.rs | 9 ++- nexus/src/populate.rs | 8 +- nexus/tests/integration_tests/schema.rs | 2 +- 21 files changed, 91 insertions(+), 121 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index bac93d057f..3adffa0118 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -231,8 +231,7 @@ 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_blocking(&db_config).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&db_config).await); // 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 08bb354fe2..34092281e4 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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 749adc2429..b14f4efee5 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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 3aa6afa9b9..cbcbb2f11c 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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 d65494b043..a50fd40dc8 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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 cc8ea6ce69..d258275005 100644 --- a/nexus/db-queries/src/db/datastore/db_metadata.rs +++ b/nexus/db-queries/src/db/datastore/db_metadata.rs @@ -511,8 +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_blocking(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg).await); let datastore = Arc::new(DataStore::new(&logctx.log, pool, None).await.unwrap()); @@ -560,8 +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_blocking(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg).await); let conn = pool.claim().await.unwrap(); // Mimic the layout of "schema/crdb". @@ -673,8 +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_blocking(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg).await); 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 3469cb6c7b..92fb98a429 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -1569,7 +1569,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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; let datastore = DataStore::new(&logctx.log, Arc::new(pool), None).await.unwrap(); let conn = datastore.pool_connection_for_tests().await.unwrap(); @@ -1618,8 +1618,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_blocking(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg).await); 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 f43d49980b..e4de883d8b 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_blocking(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg).await); 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 4104498eea..b7c8a04ab2 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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 981f626baf..773b589288 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = db::Pool::new_qorb_single_host(&cfg).await; use schema::test_users::dsl; diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index 7ff9660618..eb7103b419 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -8,7 +8,7 @@ use super::Config as DbConfig; use qorb::backend; use qorb::connectors::diesel_pg::DieselPgConnector; -use qorb::policy::{Policy, SetConfig}; +use qorb::policy::Policy; use qorb::resolver::{AllBackends, Resolver}; use qorb::resolvers::dns::{DnsResolver, DnsResolverConfig}; use qorb::service; @@ -87,9 +87,6 @@ impl Pool { /// /// Creating this pool does not necessarily wait for connections to become /// available, as backends may shift over time. - /// - /// For tests that do want to await connections being available to claim, - /// see: [Self::new_qorb_single_host_blocking]. pub fn new_qorb(bootstrap_dns: Vec) -> Self { // Make sure diesel-dtrace's USDT probes are enabled. usdt::register_probes().expect("Failed to register USDT DTrace probes"); @@ -101,61 +98,43 @@ impl Pool { Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) } } - /// Creats a new qorb-backed connection pool to a single instance of the - /// database, and waits for that backend to come online. + /// Creates a new qorb-backed connection pool to a single instance of the + /// database. + /// + /// This is intended for tests that want to skip DNS resolution, relying + /// on a single instance of the database. /// - /// This is intended for tests that want assurance that the database backend - /// is online - in production, this assumption may not be valid, and - /// [Self::new_qorb] should be preferred. - pub async fn new_qorb_single_host_blocking(db_config: &DbConfig) -> Self { + /// In production, [Self::new_qorb] should be preferred. + // TODO: Does not need to be async + pub async fn new_qorb_single_host(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(); - // NOTE: this seems like overkill, but it's actually needed - // for "test_iam_roles_behavior" -- even 50 spares doesn't work - // reliably. - // - // Unclear to me who should eat the burden for this. Should it be - // configurable? Should we let tests "await" a little longer during - // connection claiming? This wait-on-claim behavior is what bb8 did, - // but we explicitly wanted to avoid that in cases where the databases - // are offline. - let spares_wanted = 100; - let max = spares_wanted * 2; - let policy = Policy { - spares_wanted, - max_slots: max, - set_config: SetConfig { max_count: max, ..Default::default() }, - ..Default::default() - }; + let policy = Policy::default(); let pool = Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) }; - // I wish this was enough to work for our tests, but many of them try to - // grab multiple connections back-to-back. Therefore, we can't "just" - // wait for the pool to come online, we need to wait until we have some - // confidence that enough connections exist to be used by our test. - // - // Rather than flaking, we wait until unclaimed slots are made - // accessible. - // - // Since this is intended to be used by tests connecting to a single - // host locally, we SHOULD be able to make these connections. - pool.inner.block_until_online().await; - - let mut rx = pool.inner.stats().rx.clone(); - let backends = rx - .wait_for(|value| !value.is_empty()) - .await - .expect("Database never became ready and pool was dropped"); - let stats = backends.values().next().unwrap(); - - while stats.get().unclaimed_slots < spares_wanted { - tokio::time::sleep(tokio::time::Duration::from_millis(10)).await; - } + let stats = pool.inner.stats().clone(); + + tokio::task::spawn(async move { + loop { + tokio::time::sleep(tokio::time::Duration::from_secs(3)).await; + + let backends = stats.rx.borrow(); + for (name, stats) in backends.iter() { + let stats = stats.get(); + println!("Backend: {name:?}"); + println!(" Stats: {stats:?}"); + } + println!( + "Total claims: {}", + stats.claims.load(std::sync::atomic::Ordering::SeqCst) + ); + } + }); pool } diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index 4aa9ac64ac..e5b25b973d 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -917,9 +917,8 @@ 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_blocking(&cfg).await, - ); + let pool = + Arc::new(crate::db::Pool::new_qorb_single_host(&cfg).await); 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 4dc3a6a65d..8c1d7953d3 100644 --- a/nexus/db-queries/src/db/queries/next_item.rs +++ b/nexus/db-queries/src/db/queries/next_item.rs @@ -708,9 +708,7 @@ 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_blocking(&cfg).await, - ); + let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg).await); let conn = pool.claim().await.unwrap(); // We're going to operate on a separate table, for simplicity. @@ -772,9 +770,7 @@ 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_blocking(&cfg).await, - ); + let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg).await); 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 f96612a4c6..012a6a8be4 100644 --- a/nexus/db-queries/src/db/queries/region_allocation.rs +++ b/nexus/db-queries/src/db/queries/region_allocation.rs @@ -427,7 +427,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_blocking(&cfg).await; + let pool = crate::db::Pool::new_qorb_single_host(&cfg).await; 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 4af3fcb022..6650cd1cad 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_blocking(&cfg).await; + let pool = crate::db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = crate::db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = crate::db::Pool::new_qorb_single_host(&cfg).await; 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_blocking(&cfg).await; + let pool = crate::db::Pool::new_qorb_single_host(&cfg).await; 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 b0670c13c9..55126afe02 100644 --- a/nexus/db-queries/src/db/queries/vpc_subnet.rs +++ b/nexus/db-queries/src/db/queries/vpc_subnet.rs @@ -436,9 +436,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 = Arc::new( - crate::db::Pool::new_qorb_single_host_blocking(&cfg).await, - ); + let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg).await); 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 f33ebfe204..74ba637f50 100644 --- a/nexus/db-queries/src/db/saga_recovery.rs +++ b/nexus/db-queries/src/db/saga_recovery.rs @@ -327,8 +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_blocking(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg).await); 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 655ccaecbf..61ecefdf67 100644 --- a/nexus/src/bin/schema-updater.rs +++ b/nexus/src/bin/schema-updater.rs @@ -71,8 +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_blocking(&crdb_cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&crdb_cfg).await); 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 dec30832ca..7ef5a2e611 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -251,7 +251,14 @@ impl ServerContext { } }; - let pool = db::Pool::new_qorb(dns_addrs); + let pool = match &config.deployment.database { + nexus_config::Database::FromUrl { url } => { + db::Pool::new_qorb_single_host(&db::Config { url: url.clone() }) + .await + } + nexus_config::Database::FromDns => db::Pool::new_qorb(dns_addrs), + }; + let nexus = Nexus::new_with_id( rack_id, log.new(o!("component" => "nexus")), diff --git a/nexus/src/populate.rs b/nexus/src/populate.rs index ad27ba8f89..1dd18b5224 100644 --- a/nexus/src/populate.rs +++ b/nexus/src/populate.rs @@ -380,8 +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_blocking(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg).await); let datastore = Arc::new( db::DataStore::new(&logctx.log, pool, None).await.unwrap(), ); @@ -423,13 +422,12 @@ mod test { }) .unwrap(); - // Test again with the database offline.In principle we could do this + // Test again with the database offline. In principle we could do this // immediately without creating a new pool and datastore. // // 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_blocking(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg).await); // 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 f740e28f3c..dbe33b0495 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -954,7 +954,7 @@ async fn dbinit_equals_sum_of_all_up() { // Create a connection pool after we apply the first schema version but // 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_blocking( + let pool = nexus_db_queries::db::Pool::new_qorb_single_host( &nexus_db_queries::db::Config { url: crdb.pg_config().clone() }, ) .await; From 7d06d29f4900fb445df5b40226f4c3f579c7d777 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 12 Jun 2024 13:03:22 -0700 Subject: [PATCH 05/15] Integrate failfast, make constructor not async --- Cargo.lock | 2 +- Cargo.toml | 2 - dev-tools/omdb/src/bin/omdb/db.rs | 2 +- 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 | 53 ++++++++----------- .../db-queries/src/db/queries/external_ip.rs | 3 +- nexus/db-queries/src/db/queries/next_item.rs | 4 +- .../src/db/queries/region_allocation.rs | 2 +- .../virtual_provisioning_collection_update.rs | 8 +-- nexus/db-queries/src/db/queries/vpc_subnet.rs | 2 +- nexus/db-queries/src/db/saga_recovery.rs | 2 +- nexus/src/bin/schema-updater.rs | 2 +- nexus/src/context.rs | 1 - nexus/src/populate.rs | 4 +- nexus/tests/integration_tests/schema.rs | 3 +- 23 files changed, 77 insertions(+), 89 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ccb98d1b8c..5d2b70a794 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7450,7 +7450,7 @@ dependencies = [ [[package]] name = "qorb" version = "0.0.1" -source = "git+https://github.com/oxidecomputer/qorb?branch=master#64d959b94b594499e28df3117dfdeafad3ab9036" +source = "git+https://github.com/oxidecomputer/qorb?branch=master#6b39fd4c08790036ec4cc7cf18a0bcbbb0fcd34e" dependencies = [ "anyhow", "async-bb8-diesel", diff --git a/Cargo.toml b/Cargo.toml index 1cddb6d84a..b23e0d4fe5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -395,8 +395,6 @@ bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a0 propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a033babc054db9eff5b59dee978d2b0d76" } propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "6d7ed9a033babc054db9eff5b59dee978d2b0d76" } proptest = "1.4.0" -# TODO: Patch me before merging! -# qorb = { path = "../../qorb" } qorb = { git = "https://github.com/oxidecomputer/qorb", branch = "master" } quote = "1.0" rand = "0.8.5" diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index 3adffa0118..25179fe4ef 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -231,7 +231,7 @@ 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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&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 34092281e4..7fdaec5aff 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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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 b14f4efee5..661eaed6d0 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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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 cbcbb2f11c..8229f7736b 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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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 a50fd40dc8..f3bb6602ac 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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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 d258275005..f4c23b5cce 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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&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 92fb98a429..203dab7025 100644 --- a/nexus/db-queries/src/db/datastore/mod.rs +++ b/nexus/db-queries/src/db/datastore/mod.rs @@ -1569,7 +1569,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).await; + let pool = db::Pool::new_qorb_single_host(&cfg); let datastore = DataStore::new(&logctx.log, Arc::new(pool), None).await.unwrap(); let conn = datastore.pool_connection_for_tests().await.unwrap(); @@ -1618,7 +1618,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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&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 e4de883d8b..78e8bf8b31 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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&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 b7c8a04ab2..9e050a1da1 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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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 773b589288..24d6971e08 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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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).await; + let pool = db::Pool::new_qorb_single_host(&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 eb7103b419..e61656ef98 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -71,11 +71,9 @@ fn make_single_host_resolver( fn make_postgres_connector() -> qorb::backend::SharedConnector { // Create postgres connections. // - // TODO: "on acquire"? - // TODO: Connection timeout for failfast? - // - // We're currently relying on somewhat intrusive modifications to qorb to - // make these things possible. Might be worth a refactor? + // We're currently relying on the DieselPgConnector doing the following: + // - Disallowing full table scans in its implementation of "on_acquire" + // - Creating async_bb8_diesel connections that also wrap DTraceConnections. let user = "root"; let db = "omicron"; let args = Some("sslmode=disable"); @@ -105,8 +103,7 @@ impl Pool { /// on a single instance of the database. /// /// In production, [Self::new_qorb] should be preferred. - // TODO: Does not need to be async - pub async fn new_qorb_single_host(db_config: &DbConfig) -> Self { + pub fn new_qorb_single_host(db_config: &DbConfig) -> Self { // Make sure diesel-dtrace's USDT probes are enabled. usdt::register_probes().expect("Failed to register USDT DTrace probes"); @@ -114,29 +111,25 @@ impl Pool { let connector = make_postgres_connector(); let policy = Policy::default(); - let pool = - Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) }; - - let stats = pool.inner.stats().clone(); - - tokio::task::spawn(async move { - loop { - tokio::time::sleep(tokio::time::Duration::from_secs(3)).await; - - let backends = stats.rx.borrow(); - for (name, stats) in backends.iter() { - let stats = stats.get(); - println!("Backend: {name:?}"); - println!(" Stats: {stats:?}"); - } - println!( - "Total claims: {}", - stats.claims.load(std::sync::atomic::Ordering::SeqCst) - ); - } - }); - - pool + Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) } + } + + /// Creates a new qorb-backed connection pool which returns an error + /// if claims are not quickly available. + /// + /// This is intended for test-only usage. + pub fn new_qorb_single_host_failfast(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 policy = Policy { + claim_timeout: tokio::time::Duration::from_millis(1), + ..Default::default() + }; + Pool { inner: qorb::pool::Pool::new(resolver, connector, policy) } } /// Returns a connection from the pool diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index e5b25b973d..a01f6646fb 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -917,8 +917,7 @@ 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).await); + let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&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 8c1d7953d3..e25ff28eba 100644 --- a/nexus/db-queries/src/db/queries/next_item.rs +++ b/nexus/db-queries/src/db/queries/next_item.rs @@ -708,7 +708,7 @@ 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).await); + let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg)); let conn = pool.claim().await.unwrap(); // We're going to operate on a separate table, for simplicity. @@ -770,7 +770,7 @@ 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).await); + let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&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 012a6a8be4..c6bdfc5851 100644 --- a/nexus/db-queries/src/db/queries/region_allocation.rs +++ b/nexus/db-queries/src/db/queries/region_allocation.rs @@ -427,7 +427,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).await; + let pool = crate::db::Pool::new_qorb_single_host(&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 6650cd1cad..775f47a5ec 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).await; + let pool = crate::db::Pool::new_qorb_single_host(&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).await; + let pool = crate::db::Pool::new_qorb_single_host(&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).await; + let pool = crate::db::Pool::new_qorb_single_host(&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).await; + let pool = crate::db::Pool::new_qorb_single_host(&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 55126afe02..5914561d73 100644 --- a/nexus/db-queries/src/db/queries/vpc_subnet.rs +++ b/nexus/db-queries/src/db/queries/vpc_subnet.rs @@ -436,7 +436,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 = Arc::new(crate::db::Pool::new_qorb_single_host(&cfg).await); + let pool = Arc::new(crate::db::Pool::new_qorb_single_host(&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 74ba637f50..8a8768ca94 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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&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 61ecefdf67..d9ced0e18f 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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&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 7ef5a2e611..9e8f46d8f0 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -254,7 +254,6 @@ impl ServerContext { let pool = match &config.deployment.database { nexus_config::Database::FromUrl { url } => { db::Pool::new_qorb_single_host(&db::Config { url: url.clone() }) - .await } nexus_config::Database::FromDns => db::Pool::new_qorb(dns_addrs), }; diff --git a/nexus/src/populate.rs b/nexus/src/populate.rs index 1dd18b5224..3880ec2fed 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).await); + let pool = Arc::new(db::Pool::new_qorb_single_host(&cfg)); let datastore = Arc::new( db::DataStore::new(&logctx.log, pool, None).await.unwrap(), ); @@ -427,7 +427,7 @@ 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(&cfg).await); + let pool = Arc::new(db::Pool::new_qorb_single_host_failfast(&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 dbe33b0495..9352bee38a 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -956,8 +956,7 @@ async fn dbinit_equals_sum_of_all_up() { // it for an extra check later. let pool = nexus_db_queries::db::Pool::new_qorb_single_host( &nexus_db_queries::db::Config { url: crdb.pg_config().clone() }, - ) - .await; + ); let conn_from_pool = pool.claim().await.expect("failed to get pooled connection"); From 2c44ecfd09d0155a28b19c9a5a8357ecf0023519 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 12 Jun 2024 13:05:13 -0700 Subject: [PATCH 06/15] Workspace hack --- workspace-hack/Cargo.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 1b21b72495..a14c03084c 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -103,9 +103,9 @@ string_cache = { version = "0.8.7" } subtle = { version = "2.5.0" } syn-f595c2ba2a3f28df = { package = "syn", version = "2.0.64", features = ["extra-traits", "fold", "full", "visit", "visit-mut"] } time = { version = "0.3.36", features = ["formatting", "local-offset", "macros", "parsing"] } -tokio = { version = "1.37.0", features = ["full", "test-util"] } +tokio = { version = "1.38.0", features = ["full", "test-util"] } tokio-postgres = { version = "0.7.10", features = ["with-chrono-0_4", "with-serde_json-1", "with-uuid-1"] } -tokio-stream = { version = "0.1.15", features = ["net"] } +tokio-stream = { version = "0.1.15", features = ["net", "sync"] } tokio-util = { version = "0.7.11", features = ["codec", "io-util"] } toml = { version = "0.7.8" } toml_edit-3c51e837cfc5589a = { package = "toml_edit", version = "0.22.13", features = ["serde"] } @@ -209,9 +209,9 @@ syn-dff4ba8e3ae991db = { package = "syn", version = "1.0.109", features = ["extr syn-f595c2ba2a3f28df = { package = "syn", version = "2.0.64", features = ["extra-traits", "fold", "full", "visit", "visit-mut"] } time = { version = "0.3.36", features = ["formatting", "local-offset", "macros", "parsing"] } time-macros = { version = "0.2.18", default-features = false, features = ["formatting", "parsing"] } -tokio = { version = "1.37.0", features = ["full", "test-util"] } +tokio = { version = "1.38.0", features = ["full", "test-util"] } tokio-postgres = { version = "0.7.10", features = ["with-chrono-0_4", "with-serde_json-1", "with-uuid-1"] } -tokio-stream = { version = "0.1.15", features = ["net"] } +tokio-stream = { version = "0.1.15", features = ["net", "sync"] } tokio-util = { version = "0.7.11", features = ["codec", "io-util"] } toml = { version = "0.7.8" } toml_edit-3c51e837cfc5589a = { package = "toml_edit", version = "0.22.13", features = ["serde"] } From aa544fcc6ce0895b819bd24dd8e9adab70fc3f59 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 12 Jun 2024 13:10:50 -0700 Subject: [PATCH 07/15] look ma no bb8 --- Cargo.lock | 1 - Cargo.toml | 1 - nexus/db-queries/Cargo.toml | 1 - 3 files changed, 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d2b70a794..155f152f64 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4729,7 +4729,6 @@ dependencies = [ "assert_matches", "async-bb8-diesel", "async-trait", - "bb8", "camino", "camino-tempfile", "chrono", diff --git a/Cargo.toml b/Cargo.toml index b23e0d4fe5..e9779cd91c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -223,7 +223,6 @@ atomicwrites = "0.4.3" authz-macros = { path = "nexus/authz-macros" } backoff = { version = "0.4.0", features = [ "tokio" ] } base64 = "0.22.1" -bb8 = "0.8.3" bcs = "0.1.6" bincode = "1.3.3" bootstore = { path = "bootstore" } diff --git a/nexus/db-queries/Cargo.toml b/nexus/db-queries/Cargo.toml index 3d6a2ddfc4..b3bae935c2 100644 --- a/nexus/db-queries/Cargo.toml +++ b/nexus/db-queries/Cargo.toml @@ -14,7 +14,6 @@ omicron-rpaths.workspace = true anyhow.workspace = true async-bb8-diesel.workspace = true async-trait.workspace = true -bb8.workspace = true camino.workspace = true chrono.workspace = true const_format.workspace = true From d7fd53158cea4f1a6d2c87e1b3cbe51dca3a7ba1 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 12 Jun 2024 17:42:44 -0700 Subject: [PATCH 08/15] Hardcode to ignore DNS TTLs because they're currently all zero --- nexus/db-queries/src/db/pool.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index e61656ef98..10471e8906 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -58,7 +58,11 @@ fn make_dns_resolver( Box::new(DnsResolver::new( service::Name(internal_dns::ServiceName::Cockroach.srv_name()), bootstrap_dns, - DnsResolverConfig::default(), + DnsResolverConfig { + query_interval: tokio::time::Duration::from_secs(10), + hardcoded_ttl: Some(std::time::Duration::from_secs(60)), + ..Default::default() + }, )) } From 6e1dfd9589b05d7dbb07f217924da67ae0f6863f Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Wed, 12 Jun 2024 18:00:48 -0700 Subject: [PATCH 09/15] Our own custom PgConnector --- Cargo.lock | 5 +- nexus/db-queries/Cargo.toml | 2 +- nexus/db-queries/src/db/pool.rs | 3 +- nexus/db-queries/src/db/pool_connection.rs | 110 +++++++++++++++------ 4 files changed, 84 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 155f152f64..94635f4539 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7449,15 +7449,12 @@ dependencies = [ [[package]] name = "qorb" version = "0.0.1" -source = "git+https://github.com/oxidecomputer/qorb?branch=master#6b39fd4c08790036ec4cc7cf18a0bcbbb0fcd34e" +source = "git+https://github.com/oxidecomputer/qorb?branch=master#bb432c9574c393bf2eb7b19516b996aa5584573e" dependencies = [ "anyhow", - "async-bb8-diesel", "async-trait", "debug-ignore", "derive-where", - "diesel", - "diesel-dtrace", "dropshot 0.10.1", "futures", "hickory-resolver", diff --git a/nexus/db-queries/Cargo.toml b/nexus/db-queries/Cargo.toml index b3bae935c2..0b62dbdfd9 100644 --- a/nexus/db-queries/Cargo.toml +++ b/nexus/db-queries/Cargo.toml @@ -29,7 +29,7 @@ oxnet.workspace = true paste.workspace = true # See omicron-rpaths for more about the "pq-sys" dependency. pq-sys = "*" -qorb = { workspace = true, features = [ "diesel_pg", "qtop" ] } +qorb = { workspace = true, features = [ "qtop" ] } rand.workspace = true ref-cast.workspace = true schemars.workspace = true diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index 10471e8906..c6fdebedc1 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -6,8 +6,9 @@ // TODO-design Need TLS support (the types below hardcode NoTls). use super::Config as DbConfig; +use crate::db::pool_connection::DieselPgConnector; + use qorb::backend; -use qorb::connectors::diesel_pg::DieselPgConnector; use qorb::policy::Policy; use qorb::resolver::{AllBackends, Resolver}; use qorb::resolvers::dns::{DnsResolver, DnsResolverConfig}; diff --git a/nexus/db-queries/src/db/pool_connection.rs b/nexus/db-queries/src/db/pool_connection.rs index c03cbb2afa..fdf78c99dd 100644 --- a/nexus/db-queries/src/db/pool_connection.rs +++ b/nexus/db-queries/src/db/pool_connection.rs @@ -4,41 +4,91 @@ //! Customization that happens on each connection as they're acquired. +use anyhow::anyhow; +use async_bb8_diesel::AsyncR2D2Connection; +use async_bb8_diesel::AsyncSimpleConnection; +use async_trait::async_trait; +use diesel::Connection; use diesel::PgConnection; use diesel_dtrace::DTraceConnection; +use qorb::backend::{self, Backend, Error}; pub type DbConnection = DTraceConnection; pub const DISALLOW_FULL_TABLE_SCAN_SQL: &str = "set disallow_full_table_scans = on; set large_full_scan_rows = 0;"; -// /// A customizer for all new connections made to CockroachDB, from Diesel. -// #[derive(Debug)] -// pub(crate) struct ConnectionCustomizer {} -// -// impl ConnectionCustomizer { -// pub(crate) fn new() -> Self { -// Self {} -// } -// -// async fn disallow_full_table_scans( -// &self, -// conn: &mut Connection, -// ) -> Result<(), ConnectionError> { -// conn.batch_execute_async(DISALLOW_FULL_TABLE_SCAN_SQL).await?; -// Ok(()) -// } -// } -// -// #[async_trait] -// impl CustomizeConnection, ConnectionError> -// for ConnectionCustomizer -// { -// async fn on_acquire( -// &self, -// conn: &mut Connection, -// ) -> Result<(), ConnectionError> { -// self.disallow_full_table_scans(conn).await?; -// Ok(()) -// } -// } +/// A [backend::Connector] which provides access to [PgConnection]. +pub struct DieselPgConnector { + prefix: String, + suffix: String, +} + +impl DieselPgConnector { + /// Creates a new "connector" to a database, which + /// swaps out the IP address at runtime depending on the selected backend. + /// + /// Format of the url is: + /// + /// - postgresql://{user}@{address}/{db} + /// + /// Or, if arguments are supplied: + /// + /// - postgresql://{user}@{address}/{db}?{args} + pub fn new(user: &str, db: &str, args: Option<&str>) -> Self { + Self { + prefix: format!("postgresql://{user}@"), + suffix: format!( + "/{db}{}", + args.map(|args| format!("?{args}")).unwrap_or("".to_string()) + ), + } + } + + fn to_url(&self, address: std::net::SocketAddr) -> String { + format!( + "{prefix}{address}{suffix}", + prefix = self.prefix, + suffix = self.suffix, + ) + } +} + +#[async_trait] +impl backend::Connector for DieselPgConnector { + type Connection = async_bb8_diesel::Connection; + + async fn connect( + &self, + backend: &Backend, + ) -> Result { + let url = self.to_url(backend.address); + + let conn = tokio::task::spawn_blocking(move || { + let pg_conn = DbConnection::establish(&url) + .map_err(|e| Error::Other(anyhow!(e)))?; + Ok::<_, Error>(async_bb8_diesel::Connection::new(pg_conn)) + }) + .await + .expect("Task panicked establishing connection")?; + Ok(conn) + } + + async fn on_acquire( + &self, + conn: &mut Self::Connection, + ) -> Result<(), Error> { + conn.batch_execute_async(DISALLOW_FULL_TABLE_SCAN_SQL) + .await + .map_err(|e| 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 { + return Err(Error::Other(anyhow!("Connection broken"))); + } + conn.ping_async().await.map_err(|e| Error::Other(anyhow!(e))) + } +} From 88f67aa6858155b470d5550d94a2dbee98cae305 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 17 Jun 2024 14:22:26 -0700 Subject: [PATCH 10/15] Update qorb dep, more logs --- Cargo.lock | 21 ++++++++++----------- nexus/src/context.rs | 6 +++++- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8a6d08f55d..abe0ef4674 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3357,10 +3357,9 @@ dependencies = [ [[package]] name = "hubtools" -version = "0.4.6" -source = "git+https://github.com/oxidecomputer/hubtools.git?branch=main#943c4bbe6b50d1ab635d085d6204895fb4154e79" +version = "0.4.1" +source = "git+https://github.com/oxidecomputer/hubtools.git?branch=main#73cd5a84689d59ecce9da66ad4389c540d315168" dependencies = [ - "hex", "lpc55_areas", "lpc55_sign", "object 0.30.4", @@ -4274,8 +4273,8 @@ checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" [[package]] name = "lpc55_areas" -version = "0.2.5" -source = "git+https://github.com/oxidecomputer/lpc55_support#131520fc913ecce9b80557e854751953f743a7d2" +version = "0.2.4" +source = "git+https://github.com/oxidecomputer/lpc55_support#96f064eaae5e95930efaab6c29fd1b2e22225dac" dependencies = [ "bitfield", "clap", @@ -4285,8 +4284,8 @@ dependencies = [ [[package]] name = "lpc55_sign" -version = "0.3.4" -source = "git+https://github.com/oxidecomputer/lpc55_support#131520fc913ecce9b80557e854751953f743a7d2" +version = "0.3.3" +source = "git+https://github.com/oxidecomputer/lpc55_support#96f064eaae5e95930efaab6c29fd1b2e22225dac" dependencies = [ "byteorder", "const-oid", @@ -7474,7 +7473,7 @@ dependencies = [ [[package]] name = "qorb" version = "0.0.1" -source = "git+https://github.com/oxidecomputer/qorb?branch=master#bb432c9574c393bf2eb7b19516b996aa5584573e" +source = "git+https://github.com/oxidecomputer/qorb?branch=master#bacbcd46ba0a8c81034db16cdfa5becc43b617f5" dependencies = [ "anyhow", "async-trait", @@ -7490,7 +7489,7 @@ dependencies = [ "thiserror", "tokio", "tokio-stream", - "tokio-tungstenite 0.23.0", + "tokio-tungstenite 0.23.1", "tracing", ] @@ -10079,9 +10078,9 @@ dependencies = [ [[package]] name = "tokio-tungstenite" -version = "0.23.0" +version = "0.23.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "becd34a233e7e31a3dbf7c7241b38320f57393dcae8e7324b0167d21b8e320b0" +checksum = "c6989540ced10490aaf14e6bad2e3d33728a2813310a0c71d1574304c49631cd" dependencies = [ "futures-util", "log", diff --git a/nexus/src/context.rs b/nexus/src/context.rs index 9e8f46d8f0..1338625d1a 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -253,9 +253,13 @@ 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() }) } - nexus_config::Database::FromDns => db::Pool::new_qorb(dns_addrs), + nexus_config::Database::FromDns => { + info!(log, "Setting up qorb pool from DNS"; "dns_addrs" => #?dns_addrs); + db::Pool::new_qorb(dns_addrs) + } }; let nexus = Nexus::new_with_id( From 72369bb9726d6777266df148d6a2c90bba94833c Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 17 Jun 2024 15:32:28 -0700 Subject: [PATCH 11/15] 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 = From ec1f78bf3103e7dfaf81eeb08059cefc9b407757 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 17 Jun 2024 16:04:36 -0700 Subject: [PATCH 12/15] Review feedback --- dev-tools/omdb/src/bin/omdb/db.rs | 2 +- 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 | 18 ++++++++++------- nexus/db-queries/src/db/pool_connection.rs | 11 ++++++++-- .../db-queries/src/db/queries/external_ip.rs | 6 ++---- nexus/db-queries/src/db/queries/next_item.rs | 4 ++-- .../src/db/queries/region_allocation.rs | 2 +- .../virtual_provisioning_collection_update.rs | 8 ++++---- nexus/db-queries/src/db/queries/vpc_subnet.rs | 2 +- nexus/db-queries/src/db/saga_recovery.rs | 2 +- nexus/src/bin/schema-updater.rs | 2 +- nexus/src/context.rs | 4 ++-- nexus/src/populate.rs | 8 +++----- nexus/tests/integration_tests/schema.rs | 2 +- 22 files changed, 77 insertions(+), 70 deletions(-) diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index 668905f74c..d3a9d3dab3 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -232,7 +232,7 @@ impl DbUrlOptions { let db_config = db::Config { url: db_url.clone() }; let pool = - Arc::new(db::Pool::new_qorb_single_host(&log.clone(), &db_config)); + Arc::new(db::Pool::new_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 544054792b..2fac9a5b92 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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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 2cb5afbfe8..bc547d5127 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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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 fc2c342936..36755599d4 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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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 6d125a6d1a..3aaea6aeb1 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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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 c375faa2e1..b997bf384f 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(&logctx.log, &cfg)); + let pool = Arc::new(db::Pool::new_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(&logctx.log, &cfg)); + let pool = Arc::new(db::Pool::new_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(&logctx.log, &cfg)); + let pool = Arc::new(db::Pool::new_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 add420add2..643952e86c 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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg)); + let pool = Arc::new(db::Pool::new_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 98a58ebb10..bcf6a6c80f 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(&logctx.log, &cfg)); + let pool = Arc::new(db::Pool::new_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 c3e7961335..52844c204f 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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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 6d4253c055..9920440ade 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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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(&logctx.log, &cfg); + let pool = db::Pool::new_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 5dcc4a7d46..b79c9569f9 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -6,7 +6,7 @@ // TODO-design Need TLS support (the types below hardcode NoTls). use super::Config as DbConfig; -use crate::db::pool_connection::DieselPgConnector; +use crate::db::pool_connection::{DieselPgConnector, DieselPgConnectorArgs}; use qorb::backend; use qorb::policy::Policy; @@ -32,7 +32,7 @@ pub struct Pool { } // Provides an alternative to the DNS resolver for cases where we want to -// contact the pool directly. +// contact the database without performing resolution. struct SingleHostResolver { tx: watch::Sender, } @@ -85,7 +85,10 @@ fn make_postgres_connector( let user = "root"; let db = "omicron"; let args = Some("sslmode=disable"); - Arc::new(DieselPgConnector::new(log, user, db, args)) + Arc::new(DieselPgConnector::new( + log, + DieselPgConnectorArgs { user, db, args }, + )) } impl Pool { @@ -93,7 +96,7 @@ impl Pool { /// /// Creating this pool does not necessarily wait for connections to become /// available, as backends may shift over time. - pub fn new_qorb(log: &Logger, bootstrap_dns: Vec) -> Self { + pub fn new(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"); @@ -110,8 +113,8 @@ impl Pool { /// This is intended for tests that want to skip DNS resolution, relying /// on a single instance of the database. /// - /// In production, [Self::new_qorb] should be preferred. - pub fn new_qorb_single_host(log: &Logger, db_config: &DbConfig) -> Self { + /// In production, [Self::new] should be preferred. + pub fn new_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"); @@ -126,7 +129,8 @@ impl Pool { /// if claims are not quickly available. /// /// This is intended for test-only usage. - pub fn new_qorb_single_host_failfast( + #[cfg(any(test, feature = "testing"))] + pub fn new_single_host_failfast( log: &Logger, db_config: &DbConfig, ) -> Self { diff --git a/nexus/db-queries/src/db/pool_connection.rs b/nexus/db-queries/src/db/pool_connection.rs index 04b884db71..e195643008 100644 --- a/nexus/db-queries/src/db/pool_connection.rs +++ b/nexus/db-queries/src/db/pool_connection.rs @@ -20,12 +20,18 @@ pub const DISALLOW_FULL_TABLE_SCAN_SQL: &str = "set disallow_full_table_scans = on; set large_full_scan_rows = 0;"; /// A [backend::Connector] which provides access to [PgConnection]. -pub struct DieselPgConnector { +pub(crate) struct DieselPgConnector { log: Logger, prefix: String, suffix: String, } +pub(crate) struct DieselPgConnectorArgs<'a> { + pub(crate) user: &'a str, + pub(crate) db: &'a str, + pub(crate) args: Option<&'a str>, +} + impl DieselPgConnector { /// Creates a new "connector" to a database, which /// swaps out the IP address at runtime depending on the selected backend. @@ -37,7 +43,8 @@ impl DieselPgConnector { /// Or, if arguments are supplied: /// /// - postgresql://{user}@{address}/{db}?{args} - pub fn new(log: &Logger, user: &str, db: &str, args: Option<&str>) -> Self { + pub(crate) fn new(log: &Logger, args: DieselPgConnectorArgs<'_>) -> Self { + let DieselPgConnectorArgs { user, db, args } = args; Self { log: log.clone(), prefix: format!("postgresql://{user}@"), diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index b305e43724..9524545808 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -917,10 +917,8 @@ 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( - &logctx.log, - &cfg, - )); + let pool = + Arc::new(crate::db::Pool::new_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 bacc80274a..658d151a5b 100644 --- a/nexus/db-queries/src/db/queries/next_item.rs +++ b/nexus/db-queries/src/db/queries/next_item.rs @@ -709,7 +709,7 @@ mod tests { 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(&logctx.log, &cfg)); + Arc::new(crate::db::Pool::new_single_host(&logctx.log, &cfg)); let conn = pool.claim().await.unwrap(); // We're going to operate on a separate table, for simplicity. @@ -772,7 +772,7 @@ mod tests { 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(&logctx.log, &cfg)); + Arc::new(crate::db::Pool::new_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 3310ff6db6..ee11cd6ad7 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(&logctx.log, &cfg); + let pool = crate::db::Pool::new_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 ea41e7c64e..3b666a0883 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(&logctx.log, &cfg); + let pool = crate::db::Pool::new_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(&logctx.log, &cfg); + let pool = crate::db::Pool::new_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(&logctx.log, &cfg); + let pool = crate::db::Pool::new_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(&logctx.log, &cfg); + let pool = crate::db::Pool::new_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 696803217e..d716712efb 100644 --- a/nexus/db-queries/src/db/queries/vpc_subnet.rs +++ b/nexus/db-queries/src/db/queries/vpc_subnet.rs @@ -437,7 +437,7 @@ mod test { 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(&logctx.log, &cfg)); + Arc::new(crate::db::Pool::new_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 604e445a9f..149b87d0e6 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(log, &cfg)); + let pool = Arc::new(db::Pool::new_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 6e2132a75c..4a43698f00 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(&log, &crdb_cfg)); + let pool = Arc::new(db::Pool::new_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 98eb736712..ec38efcbaf 100644 --- a/nexus/src/context.rs +++ b/nexus/src/context.rs @@ -254,14 +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::Pool::new_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(&log, dns_addrs) + db::Pool::new(&log, dns_addrs) } }; diff --git a/nexus/src/populate.rs b/nexus/src/populate.rs index cbf112dd27..f64922d6c9 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(&logctx.log, &cfg)); + let pool = Arc::new(db::Pool::new_single_host(&logctx.log, &cfg)); let datastore = Arc::new( db::DataStore::new(&logctx.log, pool, None).await.unwrap(), ); @@ -427,10 +427,8 @@ 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( - &logctx.log, - &cfg, - )); + let pool = + Arc::new(db::Pool::new_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 6520c34dc8..5201b5c971 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -954,7 +954,7 @@ async fn dbinit_equals_sum_of_all_up() { // Create a connection pool after we apply the first schema version but // 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( + let pool = nexus_db_queries::db::Pool::new_single_host( log, &nexus_db_queries::db::Config { url: crdb.pg_config().clone() }, ); From dce02765357551360e80d1255bef37cfb9d6e7a6 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 17 Jun 2024 20:54:47 -0700 Subject: [PATCH 13/15] More lenient query interval, update qorb again to respect timeout inside dns, use edns --- Cargo.lock | 2 +- nexus-config/src/postgres_config.rs | 2 +- nexus/db-queries/src/db/pool.rs | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index abe0ef4674..b76b6e6436 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7473,7 +7473,7 @@ dependencies = [ [[package]] name = "qorb" version = "0.0.1" -source = "git+https://github.com/oxidecomputer/qorb?branch=master#bacbcd46ba0a8c81034db16cdfa5becc43b617f5" +source = "git+https://github.com/oxidecomputer/qorb?branch=master#7e415597cdb3a7373bb42f2cd677b29ba2c6417e" dependencies = [ "anyhow", "async-trait", diff --git a/nexus-config/src/postgres_config.rs b/nexus-config/src/postgres_config.rs index 6f60c8b4fb..0c72d2ba9e 100644 --- a/nexus-config/src/postgres_config.rs +++ b/nexus-config/src/postgres_config.rs @@ -36,7 +36,7 @@ impl PostgresConfigWithUrl { /// Accesses the first ip / port pair within the URL. /// - /// # Safety + /// # Panics /// /// This method makes the assumption that the hostname has at least one /// "host IP / port" pair which can be extracted. If the supplied URL diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index b79c9569f9..3c350dbe79 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -61,7 +61,6 @@ fn make_dns_resolver( service::Name(internal_dns::ServiceName::Cockroach.srv_name()), bootstrap_dns, DnsResolverConfig { - query_interval: tokio::time::Duration::from_secs(10), hardcoded_ttl: Some(tokio::time::Duration::MAX), ..Default::default() }, From e4d6c780fe9e6b2cc26a45dcd34ab2fc61251455 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 18 Jun 2024 15:55:15 -0700 Subject: [PATCH 14/15] Use URL crate, add comments --- Cargo.lock | 1 + nexus/db-queries/Cargo.toml | 3 +- nexus/db-queries/src/db/pool.rs | 8 +++-- nexus/db-queries/src/db/pool_connection.rs | 41 ++++++++++++++-------- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b76b6e6436..5efa665903 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4807,6 +4807,7 @@ dependencies = [ "term", "thiserror", "tokio", + "url", "usdt", "uuid", ] diff --git a/nexus/db-queries/Cargo.toml b/nexus/db-queries/Cargo.toml index 0b62dbdfd9..865869a5ae 100644 --- a/nexus/db-queries/Cargo.toml +++ b/nexus/db-queries/Cargo.toml @@ -46,8 +46,9 @@ strum.workspace = true swrite.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["full"] } -uuid.workspace = true +url.workspace = true usdt.workspace = true +uuid.workspace = true db-macros.workspace = true nexus-auth.workspace = true diff --git a/nexus/db-queries/src/db/pool.rs b/nexus/db-queries/src/db/pool.rs index 3c350dbe79..dccee6fa3f 100644 --- a/nexus/db-queries/src/db/pool.rs +++ b/nexus/db-queries/src/db/pool.rs @@ -83,7 +83,7 @@ fn make_postgres_connector( // - Creating async_bb8_diesel connections that also wrap DTraceConnections. let user = "root"; let db = "omicron"; - let args = Some("sslmode=disable"); + let args = vec![("sslmode", "disable")]; Arc::new(DieselPgConnector::new( log, DieselPgConnectorArgs { user, db, args }, @@ -125,9 +125,11 @@ impl Pool { } /// Creates a new qorb-backed connection pool which returns an error - /// if claims are not quickly available. + /// if claims are not available within one millisecond. /// - /// This is intended for test-only usage. + /// This is intended for test-only usage, in particular for tests where + /// claim requests should rapidly return errors when a backend has been + /// intentionally disabled. #[cfg(any(test, feature = "testing"))] pub fn new_single_host_failfast( log: &Logger, diff --git a/nexus/db-queries/src/db/pool_connection.rs b/nexus/db-queries/src/db/pool_connection.rs index e195643008..9a33370a5a 100644 --- a/nexus/db-queries/src/db/pool_connection.rs +++ b/nexus/db-queries/src/db/pool_connection.rs @@ -13,6 +13,7 @@ use diesel::PgConnection; use diesel_dtrace::DTraceConnection; use qorb::backend::{self, Backend, Error}; use slog::Logger; +use url::Url; pub type DbConnection = DTraceConnection; @@ -22,14 +23,15 @@ pub const DISALLOW_FULL_TABLE_SCAN_SQL: &str = /// A [backend::Connector] which provides access to [PgConnection]. pub(crate) struct DieselPgConnector { log: Logger, - prefix: String, - suffix: String, + user: String, + db: String, + args: Vec<(String, String)>, } pub(crate) struct DieselPgConnectorArgs<'a> { pub(crate) user: &'a str, pub(crate) db: &'a str, - pub(crate) args: Option<&'a str>, + pub(crate) args: Vec<(&'a str, &'a str)>, } impl DieselPgConnector { @@ -47,20 +49,29 @@ impl DieselPgConnector { let DieselPgConnectorArgs { user, db, args } = args; Self { log: log.clone(), - prefix: format!("postgresql://{user}@"), - suffix: format!( - "/{db}{}", - args.map(|args| format!("?{args}")).unwrap_or("".to_string()) - ), + user: user.to_string(), + db: db.to_string(), + args: args + .into_iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(), } } - fn to_url(&self, address: std::net::SocketAddr) -> String { - format!( - "{prefix}{address}{suffix}", - prefix = self.prefix, - suffix = self.suffix, - ) + fn to_url( + &self, + address: std::net::SocketAddr, + ) -> Result { + let user = &self.user; + let db = &self.db; + let mut url = + Url::parse(&format!("postgresql://{user}@{address}/{db}"))?; + + for (k, v) in &self.args { + url.query_pairs_mut().append_pair(k, v); + } + + Ok(url.as_str().to_string()) } } @@ -72,7 +83,7 @@ impl backend::Connector for DieselPgConnector { &self, backend: &Backend, ) -> Result { - let url = self.to_url(backend.address); + let url = self.to_url(backend.address).map_err(Error::Other)?; let conn = tokio::task::spawn_blocking(move || { let pg_conn = DbConnection::establish(&url) From 8293be55df8c9f7ff35940c7fe50403deda25da1 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Thu, 22 Aug 2024 10:08:34 -0700 Subject: [PATCH 15/15] more merging --- nexus/db-queries/src/db/queries/vpc_subnet.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/nexus/db-queries/src/db/queries/vpc_subnet.rs b/nexus/db-queries/src/db/queries/vpc_subnet.rs index 67206a9f3b..85c771c050 100644 --- a/nexus/db-queries/src/db/queries/vpc_subnet.rs +++ b/nexus/db-queries/src/db/queries/vpc_subnet.rs @@ -313,8 +313,9 @@ 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(&logctx.log, &cfg)); - let conn = pool.pool().get().await.unwrap(); + let pool = + Arc::new(crate::db::Pool::new_single_host(&logctx.log, &cfg)); + let conn = pool.claim().await.unwrap(); let explain = query.explain_async(&conn).await.unwrap(); println!("{explain}"); db.cleanup().await.unwrap(); @@ -545,7 +546,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(&logctx.log, &cfg)); + let pool = + Arc::new(crate::db::Pool::new_single_host(&logctx.log, &cfg)); let db_datastore = Arc::new( crate::db::DataStore::new(&log, Arc::clone(&pool), None) .await