Auto merge of #3922 - ferrous-systems:split-db-pool-size-env-vars, r=pietroalbini

Split DB_POOL_SIZE & DB_MIN_IDLE environment variables

This PR splits the existing environment variables `DB_POOL_SIZE` & `DB_MIN_IDLE` into separate variables to provide different connection settings to the primary & replica databases. The main reason for this change is to be able to configure different values for both database pools.

The new env variables are:

* `DB_PRIMARY_POOL_SIZE` & `DB_REPLICA_POOL_SIZE` to configure the number of connections in the primary & replica pool
* `DB_PRIMARY_MIN_IDLE` & `DB_REPLICA_MIN_IDLE` to set the number of idle connections each pool at least maintains
* update descriptions of env vars.

**Note** the descriptions of the `*_POOL_SIZE` variables in `app.json` are most likely off & need correct wording.

Author: bors

app.json

@@ -23,15 +23,25 @@
           "value": "",
           "required": false
       },
-      "DB_POOL_SIZE": {
+      "DB_PRIMARY_POOL_SIZE": {
           "value": "10",
           "required": false,
-          "description": "The maximum number of database connections managed by the pool. Set so that this value times the number of dynos is less than your connection limit."
+          "description": "The maximum number of primary database connections managed by the pool. Set so that this value times the number of dynos is less than your connection limit."
       },
-      "DB_MIN_IDLE": {
+      "DB_REPLICA_POOL_SIZE": {
+          "value": "3",
+          "required": false,
+          "description": "The maximum number of replica database connections managed by the pool. Set so that this value times the number of dynos is less than your connection limit."
+      },
+      "DB_PRIMARY_MIN_IDLE": {
           "value": "5",
           "required": false,
-          "description": "The pool will try to maintain at least this many idle connections at all times, while respecting the maximum size of the pool."
+          "description": "The pool will try to maintain at least this many idle connections on the primary database at all times, while respecting the maximum size of the pool."
+      },
+      "DB_REPLICA_MIN_IDLE": {
+          "value": "1",
+          "required": false,
+          "description": "The pool will try to maintain at least this many idle connections on the replica database at all times, while respecting the maximum size of the pool."
       },
       "DB_HELPER_THREADS": {
           "value": "3",

src/app.rs

@@ -76,18 +76,6 @@ impl App {
             ),
         );
 
-        let db_pool_size = match (dotenv::var("DB_POOL_SIZE"), config.env()) {
-            (Ok(num), _) => num.parse().expect("couldn't parse DB_POOL_SIZE"),
-            (_, Env::Production) => 10,
-            _ => 3,
-        };
-
-        let db_min_idle = match (dotenv::var("DB_MIN_IDLE"), config.env()) {
-            (Ok(num), _) => Some(num.parse().expect("couldn't parse DB_MIN_IDLE")),
-            (_, Env::Production) => Some(5),
-            _ => None,
-        };
-
         let db_helper_threads = match (dotenv::var("DB_HELPER_THREADS"), config.env()) {
             (Ok(num), _) => num.parse().expect("couldn't parse DB_HELPER_THREADS"),
             (_, Env::Production) => 3,
@@ -113,8 +101,8 @@ impl App {
             };
 
             let primary_db_config = r2d2::Pool::builder()
-                .max_size(db_pool_size)
-                .min_idle(db_min_idle)
+                .max_size(config.db.primary.pool_size)
+                .min_idle(config.db.primary.min_idle)
                 .connection_timeout(Duration::from_secs(db_connection_timeout))
                 .connection_customizer(Box::new(primary_db_connection_config))
                 .thread_pool(thread_pool.clone());
@@ -129,25 +117,25 @@ impl App {
             .unwrap()
         };
 
-        let replica_database = if let Some(url) = config.db.replica.as_ref().map(|c| &c.url) {
+        let replica_database = if let Some(pool_config) = config.db.replica.as_ref() {
             if config.use_test_database_pool {
-                Some(DieselPool::new_test(url))
+                Some(DieselPool::new_test(&pool_config.url))
             } else {
                 let replica_db_connection_config = ConnectionConfig {
                     statement_timeout: db_connection_timeout,
                     read_only: true,
                 };
 
                 let replica_db_config = r2d2::Pool::builder()
-                    .max_size(db_pool_size)
-                    .min_idle(db_min_idle)
+                    .max_size(pool_config.pool_size)
+                    .min_idle(pool_config.min_idle)
                     .connection_timeout(Duration::from_secs(db_connection_timeout))
                     .connection_customizer(Box::new(replica_db_connection_config))
                     .thread_pool(thread_pool);
 
                 Some(
                     DieselPool::new(
-                        url,
+                        &pool_config.url,
                         replica_db_config,
                         instance_metrics
                             .database_time_to_obtain_connection

src/config/database_pools.rs

@@ -2,6 +2,10 @@
 //!
 //! - `DATABASE_URL`: The URL of the postgres database to use.
 //! - `READ_ONLY_REPLICA_URL`: The URL of an optional postgres read-only replica database.
+//! - `DB_PRIMARY_POOL_SIZE`: The number of connections of the primary database.
+//! - `DB_REPLICA_POOL_SIZE`: The number of connections of the read-only / replica database.
+//! - `DB_PRIMARY_MIN_IDLE`: The primary pool will maintain at least this number of connections.
+//! - `DB_REPLICA_MIN_IDLE`: The replica pool will maintain at least this number of connections.
 //! - `DB_OFFLINE`: If set to `leader` then use the read-only follower as if it was the leader.
 //!   If set to `follower` then act as if `READ_ONLY_REPLICA_URL` was unset.
 //! - `READ_ONLY_MODE`: If defined (even as empty) then force all connections to be read-only.
@@ -20,6 +24,8 @@ pub struct DatabasePools {
 pub struct DbPoolConfig {
     pub url: String,
     pub read_only_mode: bool,
+    pub pool_size: u32,
+    pub min_idle: Option<u32>,
 }
 
 impl DatabasePools {
@@ -29,6 +35,8 @@ impl DatabasePools {
 }
 
 impl DatabasePools {
+    const DEFAULT_POOL_SIZE: u32 = 3;
+
     /// Load settings for one or more database pools from the environment
     ///
     /// # Panics
@@ -38,6 +46,27 @@ impl DatabasePools {
         let leader_url = env("DATABASE_URL");
         let follower_url = dotenv::var("READ_ONLY_REPLICA_URL").ok();
         let read_only_mode = dotenv::var("READ_ONLY_MODE").is_ok();
+
+        let primary_pool_size = match dotenv::var("DB_PRIMARY_POOL_SIZE") {
+            Ok(num) => num.parse().expect("couldn't parse DB_PRIMARY_POOL_SIZE"),
+            _ => Self::DEFAULT_POOL_SIZE,
+        };
+
+        let replica_pool_size = match dotenv::var("DB_REPLICA_POOL_SIZE") {
+            Ok(num) => num.parse().expect("couldn't parse DB_REPLICA_POOL_SIZE"),
+            _ => Self::DEFAULT_POOL_SIZE,
+        };
+
+        let primary_min_idle = match dotenv::var("DB_PRIMARY_MIN_IDLE") {
+            Ok(num) => Some(num.parse().expect("couldn't parse DB_PRIMARY_MIN_IDLE")),
+            _ => None,
+        };
+
+        let replica_min_idle = match dotenv::var("DB_REPLICA_MIN_IDLE") {
+            Ok(num) => Some(num.parse().expect("couldn't parse DB_REPLICA_MIN_IDLE")),
+            _ => None,
+        };
+
         match dotenv::var("DB_OFFLINE").as_deref() {
             // The actual leader is down, use the follower in read-only mode as the primary and
             // don't configure a replica.
@@ -46,6 +75,8 @@ impl DatabasePools {
                     url: follower_url
                         .expect("Must set `READ_ONLY_REPLICA_URL` when using `DB_OFFLINE=leader`."),
                     read_only_mode: true,
+                    pool_size: primary_pool_size,
+                    min_idle: primary_min_idle,
                 },
                 replica: None,
             },
@@ -54,20 +85,26 @@ impl DatabasePools {
                 primary: DbPoolConfig {
                     url: leader_url,
                     read_only_mode,
+                    pool_size: primary_pool_size,
+                    min_idle: primary_min_idle,
                 },
                 replica: None,
             },
             _ => Self {
                 primary: DbPoolConfig {
                     url: leader_url,
                     read_only_mode,
+                    pool_size: primary_pool_size,
+                    min_idle: primary_min_idle,
                 },
                 replica: follower_url.map(|url| DbPoolConfig {
                     url,
                     // Always enable read-only mode for the follower. In staging, we attach the
                     // same leader database to both environment variables and this ensures the
                     // connection is opened read-only even when attached to a writeable database.
                     read_only_mode: true,
+                    pool_size: replica_pool_size,
+                    min_idle: replica_min_idle,
                 }),
             },
         }
@@ -78,6 +115,8 @@ impl DatabasePools {
             primary: DbPoolConfig {
                 url: env("TEST_DATABASE_URL"),
                 read_only_mode: false,
+                pool_size: 1,
+                min_idle: None,
             },
             replica: None,
         }

src/middleware.rs

@@ -83,7 +83,7 @@ pub fn build_middleware(app: Arc<App>, endpoints: RouteBuilder) -> MiddlewareBui
     // In production we currently have 2 equally sized pools (primary and a read-only replica).
     // Because such a large portion of production traffic is for download requests (which update
     // download counts), we consider only the primary pool here.
-    if let Ok(capacity) = env::var("DB_POOL_SIZE") {
+    if let Ok(capacity) = env::var("DB_PRIMARY_POOL_SIZE") {
         if let Ok(capacity) = capacity.parse() {
             if capacity >= 10 {
                 println!(
@@ -92,7 +92,9 @@ pub fn build_middleware(app: Arc<App>, endpoints: RouteBuilder) -> MiddlewareBui
                 );
                 m.around(balance_capacity::BalanceCapacity::new(capacity))
             } else {
-                println!("BalanceCapacity middleware not enabled. DB_POOL_SIZE is too low.");
+                println!(
+                    "BalanceCapacity middleware not enabled. DB_PRIMARY_POOL_SIZE is too low."
+                );
             }
         }
     }

src/tests/server_binary.rs

@@ -88,7 +88,8 @@ impl ServerBin {
         // Bind a random port every time the server is started.
         env.insert("PORT".into(), "0".into());
         // Avoid creating too many database connections.
-        env.insert("DB_POOL_SIZE".into(), "2".into());
+        env.insert("DB_PRIMARY_POOL_SIZE".into(), "2".into());
+        env.insert("DB_REPLICA_POOL_SIZE".into(), "1".into());
         env.remove("DB_MIN_SIZE");
         // Other configuration variables needed for the application to boot.
         env.insert("WEB_ALLOWED_ORIGINS".into(), "http://localhost:8888".into());