Add an env var to allow disabling debug backtrace recording in the DB pool claim code

dev-tools/reconfigurator-exec-unsafe/src/main.rs

@@ -108,7 +108,7 @@ impl ReconfiguratorExec {
             internal_dns_resolver::QorbResolver::new(vec![self.dns_server]);
 
         info!(&log, "setting up database pool");
-        let pool = Arc::new(db::Pool::new(&log, &qorb_resolver));
+        let pool = Arc::new(db::Pool::new(&log, &qorb_resolver, true));
         let datastore = Arc::new(
             DataStore::new_failfast(&log, pool)
                 .await

live-tests/tests/common/mod.rs

@@ -134,8 +134,9 @@ async fn create_datastore(
     .context("failed to parse constructed postgres URL")?;
 
     let db_config = nexus_db_queries::db::Config { url };
-    let pool =
-        Arc::new(nexus_db_queries::db::Pool::new_single_host(log, &db_config));
+    let pool = Arc::new(nexus_db_queries::db::Pool::new_single_host(
+        log, &db_config, true,
+    ));
     DataStore::new_failfast(log, pool)
         .await
         .context("creating DataStore")

nexus-config/src/nexus_config.rs

@@ -188,12 +188,27 @@ pub struct DeploymentConfig {
     /// Configuration for HTTP clients to external services.
     #[serde(default)]
     pub external_http_clients: ExternalHttpClientConfig,
+    /// By default, we capture backtraces when claiming a connection from the DB pool, but setting
+    /// this flag to `false` will disable that behavior.
+    ///
+    /// This flag is intended as an escape hatch in case we ever encounter an unexpected
+    /// pathological case where capturing backtraces is slow enough to be an issue.
+    ///
+    /// Since we don't expect to encounter this in production, it can only currently be disabled for
+    /// debug and testing use cases. For this reason, we skip it when serializing and deseralizing
+    /// and default it to `true`.
+    #[serde(skip, default = "default_record_db_claim_backtraces")]
+    pub record_db_claim_backtraces: bool,
 }
 
 fn default_techport_external_server_port() -> u16 {
     NEXUS_TECHPORT_EXTERNAL_PORT
 }
 
+fn default_record_db_claim_backtraces() -> bool {
+    true
+}
+
 impl DeploymentConfig {
     /// Load a `DeploymentConfig` from the given TOML file
     ///

nexus/db-queries/src/db/pool.rs

@@ -51,6 +51,7 @@ pub struct Pool {
     log: Logger,
     terminated: std::sync::atomic::AtomicBool,
     quiesce: watch::Sender<Quiesce>,
+    record_db_claim_backtraces: bool,
 }
 
 // Provides an alternative to the DNS resolver for cases where we want to
@@ -116,7 +117,11 @@ impl Pool {
     ///
     /// Creating this pool does not necessarily wait for connections to become
     /// available, as backends may shift over time.
-    pub fn new(log: &Logger, resolver: &QorbResolver) -> Self {
+    pub fn new(
+        log: &Logger,
+        resolver: &QorbResolver,
+        record_db_claim_backtraces: bool,
+    ) -> Self {
         let resolver = resolver.for_service(ServiceName::Cockroach);
         let connector = make_postgres_connector(log);
         let policy = Policy::default();
@@ -135,7 +140,7 @@ impl Pool {
                 err.into_inner()
             }
         };
-        Self::new_common(inner, log.clone())
+        Self::new_common(inner, log.clone(), record_db_claim_backtraces)
     }
 
     /// Creates a new qorb-backed connection pool to a single instance of the
@@ -145,7 +150,11 @@ impl Pool {
     /// on a single instance of the database.
     ///
     /// In production, [Self::new] should be preferred.
-    pub fn new_single_host(log: &Logger, db_config: &DbConfig) -> Self {
+    pub fn new_single_host(
+        log: &Logger,
+        db_config: &DbConfig,
+        record_db_claim_backtraces: bool,
+    ) -> Self {
         let resolver = make_single_host_resolver(db_config);
         let connector = make_postgres_connector(log);
         let policy = Policy::default();
@@ -164,7 +173,7 @@ impl Pool {
                 err.into_inner()
             }
         };
-        Self::new_common(inner, log.clone())
+        Self::new_common(inner, log.clone(), record_db_claim_backtraces)
     }
 
     /// Creates a new qorb-backed connection pool to a fixed set of database
@@ -194,7 +203,7 @@ impl Pool {
                 err.into_inner()
             }
         };
-        Self::new_common(inner, log.clone())
+        Self::new_common(inner, log.clone(), true)
     }
 
     /// Creates a new qorb-backed connection pool which returns an error
@@ -229,12 +238,13 @@ impl Pool {
                 err.into_inner()
             }
         };
-        Self::new_common(inner, log.clone())
+        Self::new_common(inner, log.clone(), true)
     }
 
     fn new_common(
         inner: qorb::pool::Pool<AsyncConnection>,
         log: Logger,
+        record_db_claim_backtraces: bool,
     ) -> Self {
         let (quiesce, _) = watch::channel(Quiesce {
             new_claims_allowed: ClaimsAllowed::Allowed,
@@ -246,14 +256,21 @@ impl Pool {
             log,
             terminated: std::sync::atomic::AtomicBool::new(false),
             quiesce,
+            record_db_claim_backtraces,
         }
     }
 
     /// Returns a connection from the pool
     pub async fn claim(&self) -> Result<DataStoreConnection, Error> {
         let id = self.next_id.fetch_add(1, Ordering::Relaxed);
         let held_since = Utc::now();
-        let debug = Backtrace::force_capture().to_string();
+        // This is an escape hatch in case we ever encounter an unexpected pathological case where
+        // capturing backtraces is slow enough to be an issue:
+        let debug = if self.record_db_claim_backtraces {
+            Backtrace::force_capture().to_string()
+        } else {
+            "(backtraces disabled)".to_string()
+        };
         let allowed = self.quiesce.send_if_modified(|q| {
             if let ClaimsAllowed::Disallowed = q.new_claims_allowed {
                 false
@@ -430,7 +447,7 @@ mod test {
         let mut db = crdb::test_setup_database(log).await;
         let cfg = crate::db::Config { url: db.pg_config().clone() };
         {
-            let pool = Pool::new_single_host(&log, &cfg);
+            let pool = Pool::new_single_host(&log, &cfg, true);
             pool.terminate().await;
         }
         db.cleanup().await.unwrap();
@@ -447,7 +464,7 @@ mod test {
         let mut db = crdb::test_setup_database(log).await;
         let cfg = crate::db::Config { url: db.pg_config().clone() };
         {
-            let pool = Pool::new_single_host(&log, &cfg);
+            let pool = Pool::new_single_host(&log, &cfg, true);
             drop(pool);
         }
         db.cleanup().await.unwrap();

nexus/db-queries/src/db/pub_test_utils/mod.rs

@@ -38,7 +38,7 @@ enum Interface {
 
 fn new_pool(log: &Logger, db: &CockroachInstance) -> Arc<db::Pool> {
     let cfg = db::Config { url: db.pg_config().clone() };
-    Arc::new(db::Pool::new_single_host(log, &cfg))
+    Arc::new(db::Pool::new_single_host(log, &cfg, true))
 }
 
 struct TestDatabaseBuilder {
@@ -325,7 +325,7 @@ async fn datastore_test(
     use crate::authn;
 
     let cfg = db::Config { url: db.pg_config().clone() };
-    let pool = Arc::new(db::Pool::new_single_host(&log, &cfg));
+    let pool = Arc::new(db::Pool::new_single_host(&log, &cfg, true));
     let datastore = Arc::new(
         DataStore::new(&log, pool, None, IdentityCheckPolicy::DontCare)
             .await

nexus/src/bin/schema-updater.rs

@@ -80,7 +80,7 @@ async fn main_impl() -> anyhow::Result<()> {
     let all_versions = AllSchemaVersions::load(&schema_config.schema_dir)?;
 
     let crdb_cfg = db::Config { url: args.url };
-    let pool = Arc::new(db::Pool::new_single_host(&log, &crdb_cfg));
+    let pool = Arc::new(db::Pool::new_single_host(&log, &crdb_cfg, true));
 
     // We use the unchecked constructor of the datastore because we
     // don't want to block on someone else applying an upgrade.

nexus/src/context.rs

@@ -288,14 +288,19 @@ impl ServerContext {
                 db::Pool::new_single_host(
                     &log,
                     &db::Config { url: url.clone() },
+                    config.deployment.record_db_claim_backtraces,
                 )
             }
             nexus_config::Database::FromDns => {
                 info!(
                     log, "Setting up qorb database pool from DNS";
                     "dns_addrs" => ?qorb_resolver.bootstrap_dns_ips(),
                 );
-                db::Pool::new(&log, &qorb_resolver)
+                db::Pool::new(
+                    &log,
+                    &qorb_resolver,
+                    config.deployment.record_db_claim_backtraces,
+                )
             }
         };
 

nexus/src/populate.rs

@@ -367,7 +367,7 @@ mod test {
         let logctx = dev::test_setup_log("test_populator");
         let db = TestDatabase::new_populate_schema_only(&logctx.log).await;
         let cfg = db::Config { url: db.crdb().pg_config().clone() };
-        let pool = Arc::new(db::Pool::new_single_host(&logctx.log, &cfg));
+        let pool = Arc::new(db::Pool::new_single_host(&logctx.log, &cfg, true));
         let datastore = Arc::new(
             db::DataStore::new(
                 &logctx.log,

nexus/test-utils/src/starter.rs

@@ -117,6 +117,7 @@ use slog::{Logger, debug, error, info, o};
 use std::collections::BTreeMap;
 use std::collections::BTreeSet;
 use std::collections::HashMap;
+use std::env;
 use std::fmt::Debug;
 use std::iter::{once, repeat, zip};
 use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV6};
@@ -203,6 +204,12 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> {
 
         let debug_dropbox_dir = TestTempDir::new(&logctx.log);
 
+        // Note that the record_db_claim_backtraces flag defaults to true; we only disable the flag
+        // specifically when this env var is set to "0":
+        if env::var_os("NEXUS_ENABLE_DB_CLAIM_BACKTRACES") == Some("0".into()) {
+            config.deployment.record_db_claim_backtraces = false;
+        }
+
         Self {
             config,
             test_name,

sled-agent/src/services.rs

@@ -2282,6 +2282,7 @@ impl ServiceManager {
                             treat_loopback_as_external:
                                 nexus_config::TreatLoopbackAsExternal::No,
                         },
+                    record_db_claim_backtraces: true,
                 };
 
                 // Copy the partial config file to the expected