One backup reader and a spread start checkpoint by default; --start-fast when it would outlast db-timeout

README.md

@@ -98,8 +98,9 @@ Image-level tuning knobs:
 | `WAL_DROP_THRESHOLD_MB` | `pg_wal/` size at which the archive-push wrapper drops failing segments to keep Postgres running (default `5120`, matching `archive-push-queue-max`). Outside the `PGBACKREST_*` namespace because pgBackRest treats unknown `PGBACKREST_*` vars as config options and warns about them on every push. |
 | `PGBACKREST_ARCHIVE_PUSH_PROCESS_MAX` | parallel workers for `archive-push`. Default auto-sized as `clamp(cpus/8, 2, 8)`. |
 | `PGBACKREST_ARCHIVE_GET_PROCESS_MAX` | parallel workers for `archive-get`. Default auto-sized as `clamp(cpus/8, 2, 8)`. |
-| `PGBACKREST_BACKUP_PROCESS_MAX` | parallel workers for `backup`. Default auto-sized as `clamp(cpus/4, 1, 2)`. Set `1` to reduce backup concurrency independently of archiving and restores. |
+| `PGBACKREST_BACKUP_PROCESS_MAX` | parallel workers for `backup`. Default `1`, independent of vCPU, to reduce contention with live queries on IOPS-limited volumes. Explicit overrides still win. |
 | `PGBACKREST_RESTORE_PROCESS_MAX` | parallel workers for `restore`. Default auto-sized as `clamp(cpus, 1, 32)`. |
+| `PGBACKREST_START_FAST` | native pgBackRest option. Default `n` in the image config: spread the backup-start checkpoint instead of forcing a fast one. Set `y` to opt into the previous behavior. Setting it either way also turns off the watcher's own decision below. |
 
 The four worker overrides are template settings, not native pgBackRest
 environment options. `patroni-runner` renders them into command-specific
@@ -108,6 +109,23 @@ config sections. The image's `pgbackrest` launcher removes these variables
 environment so unknown-option warnings cannot corrupt `info --output=json`.
 Native options such as `PGBACKREST_REPO1_PATH` remain available.
 
+Full and differential backups use one worker and `start-fast=n` by default.
+This reduces reader concurrency and checkpoint write bursts; it is not a hard
+IOPS cap, and backups can take longer to start and finish. WAL shipping,
+archive-get, restore worker counts, backup cadence and retention are
+unchanged. Redeploy to pick up the new defaults.
+
+Before each backup the watcher reads `checkpoint_timeout` and
+`checkpoint_completion_target` from the server. A backup opened without
+`start-fast` waits for a spread checkpoint, up to about twice
+`checkpoint_completion_target × checkpoint_timeout` when one is already
+running, with no byte progress for the stall watchdog to see and under
+pgBackRest's 30-minute `db-timeout`. When that worst case reaches the lower
+of the two, the watcher adds `--start-fast` (one immediate checkpoint) and
+logs why, instead of letting every backup time out. `PGBACKREST_START_FAST`
+(`y` or `n`) turns the heuristic off; pgBackRest reads that variable directly
+and it wins over the config file.
+
 When `WAL_ARCHIVE_BUCKET` is set, `patroni-runner` writes
 `archive_mode=on`, `archive_command='/usr/local/bin/pgbackrest-archive-push-wrapper.sh %p'`,
 and `archive_timeout` (default `60`, override via `POSTGRES_ARCHIVE_TIMEOUT`)

postgres-patroni/src/patroni/backup_watcher.rs

@@ -209,7 +209,8 @@ struct StallConfig {
     /// `WAL_BACKUP_STALL_SECONDS` (default 1800 = 30 min; 0 disables the
     /// watchdog): the floor of the window. It covers the phases in which
     /// pgBackRest reports no byte progress at all — pg_backup_start
-    /// (start-fast=y: one immediate checkpoint), removing a non-resumable
+    /// (start-fast=n: a spread checkpoint, or an immediate one when
+    /// `backup_start_fast_arg` adds --start-fast), removing a non-resumable
     /// earlier attempt from the bucket, building and saving the manifest, and
     /// the tail after the last progress write (pg_backup_stop plus the
     /// archive-timeout wait for the closing WAL). Seconds to minutes on a
@@ -244,6 +245,117 @@ impl StallConfig {
     }
 }
 
+/// pgBackRest's `db-timeout` default, in seconds; it bounds `pg_backup_start`.
+const PGBACKREST_DB_TIMEOUT_DEFAULT_SECONDS: u64 = 1800;
+
+/// `PGBACKREST_DB_TIMEOUT` as seconds: a plain number or one with an s/m/h
+/// suffix (what pgBackRest accepts). Anything else is `None`.
+fn parse_db_timeout_seconds(raw: Option<&str>) -> Option<u64> {
+    let raw = raw?.trim();
+    let (num, mult) = if let Some(n) = raw.strip_suffix('h') {
+        (n, 3600)
+    } else if let Some(n) = raw.strip_suffix('m') {
+        (n, 60)
+    } else {
+        (raw.strip_suffix('s').unwrap_or(raw), 1)
+    };
+    num.parse::<u64>().ok().map(|n| n * mult)
+}
+
+/// Bound the backup-start wait must stay under: pgBackRest's db-timeout or
+/// the stall watchdog floor, whichever is lower. A disabled watchdog (0)
+/// does not bound it.
+fn backup_start_wait_limit_seconds(db_timeout_env: Option<&str>, stall_floor_seconds: u64) -> u64 {
+    let db_timeout =
+        parse_db_timeout_seconds(db_timeout_env).unwrap_or(PGBACKREST_DB_TIMEOUT_DEFAULT_SECONDS);
+    if stall_floor_seconds > 0 {
+        db_timeout.min(stall_floor_seconds)
+    } else {
+        db_timeout
+    }
+}
+
+/// Pure verdict: a spread backup-start checkpoint that could wait
+/// `wait_seconds` needs `--start-fast` under a `limit_seconds` bound.
+fn needs_start_fast(wait_seconds: u64, limit_seconds: u64) -> bool {
+    wait_seconds >= limit_seconds
+}
+
+/// Worst-case seconds `pg_backup_start(fast => false)` can wait on this
+/// server: 2 × checkpoint_completion_target × checkpoint_timeout, rounded
+/// up. Without start-fast Postgres spreads the backup-start checkpoint over
+/// completion_target × checkpoint_timeout however little is dirty (measured
+/// on 16: 275 MB dirty, checkpoint_timeout=60s → 55 s wait, 0.5 s with
+/// fast), and a request that lands while a spread checkpoint is already
+/// running waits for that one and then for its own.
+async fn spread_checkpoint_wait_seconds() -> Result<u64> {
+    let out = Command::new("psql")
+        .args([
+            "-U",
+            "postgres",
+            "-h",
+            "/var/run/postgresql",
+            "-tAXq",
+            "-c",
+            "SELECT ceil(2 * t.setting::numeric * c.setting::numeric)::bigint \
+             FROM pg_settings t, pg_settings c \
+             WHERE t.name = 'checkpoint_timeout' \
+             AND c.name = 'checkpoint_completion_target'",
+        ])
+        .env_remove("PGHOST")
+        .env_remove("PGPORT")
+        .output()
+        .await?;
+    if !out.status.success() {
+        anyhow::bail!(
+            "checkpoint settings query failed: {}",
+            String::from_utf8_lossy(&out.stderr)
+        );
+    }
+    let text = String::from_utf8_lossy(&out.stdout);
+    text.trim()
+        .parse::<u64>()
+        .map_err(|e| anyhow::anyhow!("checkpoint settings query returned {:?}: {e}", text.trim()))
+}
+
+/// Extra `pgbackrest backup` argument for how the backup is opened.
+///
+/// `pg_backup_start` runs under pgBackRest's db-timeout (1800 s) and the
+/// stall watchdog sees no byte progress while it waits for the checkpoint,
+/// so a checkpoint_timeout long enough to push the worst case past either
+/// limit would make every backup fail before it copies a byte. When the
+/// server's settings say so this returns `--start-fast` (one immediate
+/// checkpoint) and logs why. An operator who set `PGBACKREST_START_FAST` has
+/// decided already: pgBackRest reads that variable directly and it wins
+/// over the config file, so the watcher stays out of it. Unknown settings
+/// (a failed query) change nothing. Mirrors postgres-ssl's
+/// `decide_backup_start_fast`.
+async fn backup_start_fast_arg(cfg: &StallConfig) -> Option<&'static str> {
+    if env::var_os("PGBACKREST_START_FAST").is_some() {
+        return None;
+    }
+    let wait = match spread_checkpoint_wait_seconds().await {
+        Ok(w) => w,
+        Err(e) => {
+            debug!(error = %e, "pgbackrest-watcher: backup start: checkpoint settings unavailable; starting as configured");
+            return None;
+        }
+    };
+    let limit = backup_start_wait_limit_seconds(
+        env::var("PGBACKREST_DB_TIMEOUT").ok().as_deref(),
+        cfg.floor_seconds,
+    );
+    if !needs_start_fast(wait, limit) {
+        return None;
+    }
+    info!(
+        wait_seconds = wait,
+        limit_seconds = limit,
+        "pgbackrest-watcher: backup start: a spread checkpoint could wait up to {wait}s (2 x checkpoint_completion_target x checkpoint_timeout), reaching the {limit}s backup-start limit; starting with --start-fast (set PGBACKREST_START_FAST=y or n to decide explicitly)"
+    );
+    Some("--start-fast")
+}
+
 impl WatcherConfig {
     fn from_env() -> Self {
         let full_hours = env_u64("WAL_BACKUP_FULL_INTERVAL_HOURS", 168);
@@ -2646,13 +2758,18 @@ async fn run_backup_supervised(
     backup_type: &str,
     cfg: &StallConfig,
 ) -> std::io::Result<std::process::ExitStatus> {
+    let type_arg = format!("--type={backup_type}");
+    let mut args = vec![
+        "--stanza=main",
+        "backup",
+        type_arg.as_str(),
+        "--no-expire-auto",
+    ];
+    if let Some(flag) = backup_start_fast_arg(cfg).await {
+        args.push(flag);
+    }
     let mut child = Command::new("pgbackrest")
-        .args([
-            "--stanza=main",
-            "backup",
-            &format!("--type={backup_type}"),
-            "--no-expire-auto",
-        ])
+        .args(&args)
         .env_remove("PGHOST")
         .env_remove("PGPORT")
         .spawn()?;
@@ -4062,6 +4179,45 @@ P00   INFO: stanza-create command end: aborted with exception [055]\n";
     }
 }
 
+#[cfg(test)]
+mod start_fast_tests {
+    use super::{backup_start_wait_limit_seconds, needs_start_fast, parse_db_timeout_seconds};
+
+    #[test]
+    fn db_timeout_parses_seconds_and_suffixes() {
+        assert_eq!(parse_db_timeout_seconds(Some("1800")), Some(1800));
+        assert_eq!(parse_db_timeout_seconds(Some("900s")), Some(900));
+        assert_eq!(parse_db_timeout_seconds(Some("45m")), Some(2700));
+        assert_eq!(parse_db_timeout_seconds(Some("2h")), Some(7200));
+        assert_eq!(parse_db_timeout_seconds(None), None);
+        assert_eq!(parse_db_timeout_seconds(Some("")), None);
+        assert_eq!(parse_db_timeout_seconds(Some("m")), None);
+        assert_eq!(parse_db_timeout_seconds(Some("1800.5")), None);
+    }
+
+    #[test]
+    fn limit_is_the_lower_of_db_timeout_and_stall_floor() {
+        assert_eq!(backup_start_wait_limit_seconds(None, 1800), 1800);
+        assert_eq!(backup_start_wait_limit_seconds(None, 600), 600);
+        assert_eq!(backup_start_wait_limit_seconds(None, 0), 1800);
+        assert_eq!(backup_start_wait_limit_seconds(Some("1h"), 0), 3600);
+        assert_eq!(backup_start_wait_limit_seconds(Some("1h"), 1800), 1800);
+        assert_eq!(backup_start_wait_limit_seconds(Some("junk"), 0), 1800);
+    }
+
+    #[test]
+    fn start_fast_once_the_spread_wait_reaches_the_limit() {
+        // 2 x 0.9 x checkpoint_timeout: 5min -> 540, 16min -> 1728, 17min -> 1836.
+        assert!(!needs_start_fast(540, 1800));
+        assert!(!needs_start_fast(1728, 1800));
+        assert!(needs_start_fast(1836, 1800));
+        assert!(needs_start_fast(2160, 1800));
+        // A 600 s stall floor bounds a 400s checkpoint_timeout (720) but not 5min.
+        assert!(!needs_start_fast(540, 600));
+        assert!(needs_start_fast(720, 600));
+    }
+}
+
 #[cfg(test)]
 mod stall_tests {
     use super::{

postgres-patroni/src/wal_archive.rs

@@ -283,6 +283,30 @@ pub fn env_or_clamp(var: &str, default: u32) -> u32 {
     parse_process_max(env::var(var).ok().as_deref(), default)
 }
 
+/// Per-command `process-max` defaults for a container with `cpus` vCPU,
+/// before the `PGBACKREST_*_PROCESS_MAX` overrides. Pure so the sizing is
+/// unit-testable without touching env.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub struct ProcessMaxDefaults {
+    pub push: u32,
+    pub get: u32,
+    pub backup: u32,
+    pub restore: u32,
+}
+
+/// archive-push / archive-get: `clamp(cpus/8, 2, 8)`. backup: one reader,
+/// whatever the vCPU count — volume IOPS do not scale with vCPU and every
+/// extra reader competes with live queries for the same disk (mirrors
+/// postgres-ssl #151). restore: the DB is down, so `clamp(cpus, 1, 32)`.
+pub fn process_max_defaults(cpus: i64) -> ProcessMaxDefaults {
+    ProcessMaxDefaults {
+        push: clamp(cpus / 8, 2, 8),
+        get: clamp(cpus / 8, 2, 8),
+        backup: 1,
+        restore: clamp(cpus, 1, 32),
+    }
+}
+
 /// pg_wal drop ceiling (MiB) and pgBackRest archive-push spool ceiling (MiB).
 /// Both scale DOWN from the absolute default (5120) on small volumes — never
 /// up. On volumes ≥10 GiB the absolute holds.
@@ -345,10 +369,11 @@ pub fn render_pgbackrest_conf(data_dir: &str, queue_max_mib: u32) -> Result<()>
     let conf_path = "/etc/pgbackrest/pgbackrest.conf";
 
     let cpus = detect_cpus().max(1) as i64;
-    let push_max = env_or_clamp("PGBACKREST_ARCHIVE_PUSH_PROCESS_MAX", clamp(cpus / 8, 2, 8));
-    let get_max = env_or_clamp("PGBACKREST_ARCHIVE_GET_PROCESS_MAX", clamp(cpus / 8, 2, 8));
-    let backup_max = env_or_clamp("PGBACKREST_BACKUP_PROCESS_MAX", clamp(cpus / 4, 1, 2));
-    let restore_max = env_or_clamp("PGBACKREST_RESTORE_PROCESS_MAX", clamp(cpus, 1, 32));
+    let defaults = process_max_defaults(cpus);
+    let push_max = env_or_clamp("PGBACKREST_ARCHIVE_PUSH_PROCESS_MAX", defaults.push);
+    let get_max = env_or_clamp("PGBACKREST_ARCHIVE_GET_PROCESS_MAX", defaults.get);
+    let backup_max = env_or_clamp("PGBACKREST_BACKUP_PROCESS_MAX", defaults.backup);
+    let restore_max = env_or_clamp("PGBACKREST_RESTORE_PROCESS_MAX", defaults.restore);
 
     info!(
         cpus = cpus,
@@ -442,7 +467,7 @@ fn build_pgbackrest_conf(params: &PgbackrestConfParams) -> String {
          spool-path={spool_dir}\n\
          compress-type=zst\n\
          compress-level=3\n\
-         start-fast=y\n\
+         start-fast=n\n\
          \n\
          [global:archive-push]\n\
          process-max={push_max}\n\
@@ -736,6 +761,41 @@ mod tests {
         }
     }
 
+    #[test]
+    fn backup_process_max_defaults_to_one_at_every_cpu_size() {
+        // Volume IOPS do not scale with vCPU: the backup reader count stays
+        // at 1 while the other commands keep their cpu-derived defaults.
+        for cpus in [1i64, 4, 16, 64, 256] {
+            let d = process_max_defaults(cpus);
+            assert_eq!(d.backup, 1, "cpus={cpus}");
+            assert_eq!(d.push, clamp(cpus / 8, 2, 8), "cpus={cpus}");
+            assert_eq!(d.get, clamp(cpus / 8, 2, 8), "cpus={cpus}");
+            assert_eq!(d.restore, clamp(cpus, 1, 32), "cpus={cpus}");
+        }
+        // Explicit overrides still win and stay command-scoped.
+        assert_eq!(
+            parse_process_max(Some("4"), process_max_defaults(256).backup),
+            4
+        );
+    }
+
+    #[test]
+    fn pgbackrest_conf_spreads_the_backup_start_checkpoint() {
+        let conf = build_pgbackrest_conf(&PgbackrestConfParams {
+            data_dir: "/pgdata",
+            queue_max_mib: 5120,
+            push_max: 2,
+            get_max: 2,
+            backup_max: 1,
+            restore_max: 4,
+            retention_full: 4,
+            retention_diff: 14,
+        });
+        assert!(conf.contains("[global]\n"));
+        assert!(conf.contains("\nstart-fast=n\n"), "{conf}");
+        assert!(!conf.contains("start-fast=y"), "{conf}");
+    }
+
     #[test]
     fn pgbackrest_conf_renders_all_five_process_max_sections() {
         let conf = build_pgbackrest_conf(&PgbackrestConfParams {