Auto merge of #4645 - ferrous-systems:pa-tcp-user-timeout, r=Turbo87

Set `TCP_USER_TIMEOUT` to 15 seconds for database connections

In the December 22, 2021 crates.io outage, we experienced full packet loss between the database server and the application server(s). While the crates.io application supports running without a database during outages, those mitigations kicked in only after a bit more than 15 minutes of the packet loss starting.

The default parameters of the Linux network stack result in a TCP connection being marked as broken after roughly 15 minutes of no acknowledgements being received, which is what I believe happened during that outage.  Broken database mitigations kicking in after 15 minutes is too long for crates.io, as we ideally want those mitigations to kick in as soon as possible (ensuring crates.io users can continue downloading crates).

This PR tells libpq (the underlying PostgreSQL client used by diesel) to set the Linux network stack timeout to a configurable value, which we set as **15 seconds**. With this change, if an outage like the one on Dec 22 2021 one happens again, crates.io will be fully unavailable only for 15 seconds rather than 15 minutes.

Note that the "15 seconds" value is a hunch I had, I'm open on feedback on different values we could use.

This PR intentionally doesn't have any tests: while I have a local commit changing ChaosProxy to simulate packet loss, that's done by shelling out to `iptables`, which is both linux-only and root-only. I am not aware of any cross-platform, unprivileged tool we could use to simulate this in our test suite unfortunately.

Huge thanks to `@weiznich` in diesel-rs/diesel#3016 for helping debug this issue and for trying possible fixes on the diesel side.

Author: bors

src/admin/delete_crate.rs

@@ -14,7 +14,7 @@ pub struct Opts {
 }
 
 pub fn run(opts: Opts) {
-    let conn = db::connect_now().unwrap();
+    let conn = db::oneoff_connection().unwrap();
     conn.transaction::<_, diesel::result::Error, _>(|| {
         delete(opts, &conn);
         Ok(())

src/admin/delete_version.rs

@@ -21,7 +21,7 @@ pub struct Opts {
 }
 
 pub fn run(opts: Opts) {
-    let conn = db::connect_now().unwrap();
+    let conn = db::oneoff_connection().unwrap();
     conn.transaction::<_, diesel::result::Error, _>(|| {
         delete(opts, &conn);
         Ok(())

src/admin/migrate.rs

@@ -11,13 +11,15 @@ diesel_migrations::embed_migrations!("./migrations");
 pub struct Opts;
 
 pub fn run(_opts: Opts) -> Result<(), Error> {
-    let db_config = crate::config::DatabasePools::full_from_environment();
+    let config = crate::config::DatabasePools::full_from_environment(
+        &crate::config::Base::from_environment(),
+    );
 
     // TODO: Refactor logic so that we can also check things from App::new() here.
     // If the app will panic due to bad configuration, it is better to error in the release phase
     // to avoid launching dynos that will fail.
 
-    if db_config.are_all_read_only() {
+    if config.are_all_read_only() {
         // TODO: Check `any_pending_migrations()` with a read-only connection and error if true.
         // It looks like this requires changes upstream to make this pub in `migration_macros`.
 
@@ -29,13 +31,14 @@ pub fn run(_opts: Opts) -> Result<(), Error> {
         return Ok(());
     }
 
-    println!("==> migrating the database");
     // The primary is online, access directly via `DATABASE_URL`.
-    let conn = crate::db::connect_now()?;
+    let conn = crate::db::oneoff_connection_with_config(&config)?;
+
+    println!("==> migrating the database");
     embedded_migrations::run_with_output(&conn, &mut std::io::stdout())?;
 
     println!("==> synchronizing crate categories");
-    crate::boot::categories::sync(CATEGORIES_TOML).unwrap();
+    crate::boot::categories::sync_with_connection(CATEGORIES_TOML, &conn).unwrap();
 
     Ok(())
 }

src/admin/populate.rs

@@ -14,7 +14,7 @@ pub struct Opts {
 }
 
 pub fn run(opts: Opts) {
-    let conn = db::connect_now().unwrap();
+    let conn = db::oneoff_connection().unwrap();
     conn.transaction(|| update(opts, &conn)).unwrap();
 }
 

src/admin/render_readmes.rs

@@ -40,7 +40,7 @@ pub struct Opts {
 
 pub fn run(opts: Opts) -> anyhow::Result<()> {
     let base_config = Arc::new(config::Base::from_environment());
-    let conn = db::connect_now().unwrap();
+    let conn = db::oneoff_connection().unwrap();
 
     let start_time = Utc::now();
 

src/admin/transfer_crates.rs

@@ -21,7 +21,7 @@ pub struct Opts {
 }
 
 pub fn run(opts: Opts) {
-    let conn = db::connect_now().unwrap();
+    let conn = db::oneoff_connection().unwrap();
     conn.transaction::<_, diesel::result::Error, _>(|| {
         transfer(opts, &conn);
         Ok(())

src/admin/verify_token.rs

@@ -13,7 +13,7 @@ pub struct Opts {
 }
 
 pub fn run(opts: Opts) -> AppResult<()> {
-    let conn = db::connect_now()?;
+    let conn = db::oneoff_connection()?;
     let user = User::find_by_api_token(&conn, &opts.api_token)?;
     println!("The token belongs to user {}", user.gh_login);
     Ok(())

src/app.rs

@@ -100,7 +100,7 @@ impl App {
         let thread_pool = Arc::new(ScheduledThreadPool::new(db_helper_threads));
 
         let primary_database = if config.use_test_database_pool {
-            DieselPool::new_test(&config.db.primary.url)
+            DieselPool::new_test(&config.db, &config.db.primary.url)
         } else {
             let primary_db_connection_config = ConnectionConfig {
                 statement_timeout: db_connection_timeout,
@@ -116,6 +116,7 @@ impl App {
 
             DieselPool::new(
                 &config.db.primary.url,
+                &config.db,
                 primary_db_config,
                 instance_metrics
                     .database_time_to_obtain_connection
@@ -126,7 +127,7 @@ impl App {
 
         let replica_database = if let Some(pool_config) = config.db.replica.as_ref() {
             if config.use_test_database_pool {
-                Some(DieselPool::new_test(&pool_config.url))
+                Some(DieselPool::new_test(&config.db, &pool_config.url))
             } else {
                 let replica_db_connection_config = ConnectionConfig {
                     statement_timeout: db_connection_timeout,
@@ -143,6 +144,7 @@ impl App {
                 Some(
                     DieselPool::new(
                         &pool_config.url,
+                        &config.db,
                         replica_db_config,
                         instance_metrics
                             .database_time_to_obtain_connection

src/bin/background-worker.rs

@@ -24,11 +24,10 @@ use std::time::Duration;
 fn main() {
     println!("Booting runner");
 
-    let db_config = config::DatabasePools::full_from_environment();
-    let base_config = config::Base::from_environment();
-    let uploader = base_config.uploader();
+    let config = config::Server::default();
+    let uploader = config.base.uploader();
 
-    if db_config.are_all_read_only() {
+    if config.db.are_all_read_only() {
         loop {
             println!(
                 "Cannot run background jobs with a read-only pool. Please scale background_worker \
@@ -38,7 +37,7 @@ fn main() {
         }
     }
 
-    let db_url = db::connection_url(&db_config.primary.url);
+    let db_url = db::connection_url(&config.db, &config.db.primary.url);
 
     let job_start_timeout = dotenv::var("BACKGROUND_JOB_TIMEOUT")
         .unwrap_or_else(|_| "30".into())

src/bin/enqueue-job.rs

@@ -7,7 +7,7 @@ use swirl::schema::background_jobs::dsl::*;
 use swirl::Job;
 
 fn main() -> Result<()> {
-    let conn = db::connect_now()?;
+    let conn = db::oneoff_connection()?;
     let mut args = std::env::args().skip(1);
 
     let job = args.next().unwrap_or_default();