fix(backup): compressed -Fc dumps + pg_restore; reconcile subprocess timeouts (#739)

DB backup polish (plan-task #764 Q3):
- pg_dump now uses custom format (-Fc): compressed (much smaller on NFS) and
  restored via pg_restore. Artifact extension .sql → .dump; restore_db swaps
  psql -f for pg_restore -d. BackupRun.sql_path field name kept (it's just the
  db artifact path).
- Reconcile the subprocess guardrails: the DB timeout was 720s with a stale
  'Celery soft is 10 min' comment, but backup_db_task's soft limit is actually
  1800s — so the bounded-kill fired 18 min early. Set DB=1700s / images=21000s,
  each just under its task's Celery soft limit so _run_bounded stays the
  primary guard (an NFS D-state hang defeats Celery's own SIGKILL).

Real shrink of the DB is the #764 prune; this makes each dump smaller/faster
on top of that.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

backend/app/services/backup_service.py

@@ -24,11 +24,17 @@ from pathlib import Path
 
 _BACKUPS_DIRNAME = "_backups"
 
-# Subprocess-level guardrails BEYOND the Celery soft_time_limit. The
-# Celery soft limit signals the Python process; subprocess.Popen in a
-# blocking syscall ignores that signal. These bound the worst case.
-_DB_SUBPROCESS_TIMEOUT_S = 12 * 60        # 12 min (Celery soft is 10 min)
-_IMAGES_SUBPROCESS_TIMEOUT_S = 7 * 60 * 60  # 7 hr (Celery soft is 6 hr)
+# Subprocess-level guardrails BEYOND the Celery soft_time_limit. The Celery
+# soft limit signals the Python process; subprocess.Popen in a blocking syscall
+# ignores that signal, so these bound the worst case directly. Each sits just
+# UNDER its task's Celery soft_time_limit so the bounded-kill (_run_bounded) is
+# the primary guard and fires cleanly before Celery's soft/hard limits — which
+# matters because an NFS D-state hang defeats even Celery's SIGKILL (the failure
+# that wedged the maintenance lane for hours, #739).
+#   backup_db_task:     soft=1800s / hard=2100s  → 1700s
+#   backup_images_task: soft=21600s / hard=23400s → 21000s
+_DB_SUBPROCESS_TIMEOUT_S = 1700            # ~28 min, under the 30-min DB soft limit
+_IMAGES_SUBPROCESS_TIMEOUT_S = 21000       # ~5.8 hr, under the 6-hr images soft limit
 # Grace after SIGKILL to reap the child. If it can't be reaped in this window
 # (an uninterruptible NFS D-state — the failure mode that wedged the
 # concurrency-1 maintenance lane for hours, operator-flagged 2026-06-07), we
@@ -115,18 +121,21 @@ def backup_db(
     to persist into BackupRun. Raises on subprocess failure."""
     ts = _now_ts()
     out_dir = _backups_dir(images_root)
-    sql_path = out_dir / f"fc_db_{ts}.sql"
+    # Custom format (-Fc): compressed (much smaller on NFS) and restored with
+    # pg_restore. The .dump extension marks it as non-SQL. The BackupRun field
+    # is still named sql_path — it's just "the db artifact path".
+    sql_path = out_dir / f"fc_db_{ts}.dump"
     # Dump to LOCAL disk first, then move the finished file to the (NFS) backups
     # dir. pg_dump's long phase is then a DB-socket wait + local writes — both
     # killable — instead of an NFS write that can hang uninterruptibly. Only the
     # final move touches NFS, and it's a bounded single-file step.
-    fd, tmp_name = tempfile.mkstemp(prefix="fc_db_", suffix=".sql")
+    fd, tmp_name = tempfile.mkstemp(prefix="fc_db_", suffix=".dump")
     os.close(fd)
     tmp_path = Path(tmp_name)
     try:
         _run_bounded(
             [
-                "pg_dump", "--no-owner", "--no-acl",
+                "pg_dump", "--no-owner", "--no-acl", "-Fc",
                 "-f", str(tmp_path), _libpq_url(db_url),
             ],
             _DB_SUBPROCESS_TIMEOUT_S,
@@ -184,8 +193,8 @@ def backup_images(
 
 
 def restore_db(*, db_url: str, sql_path: Path) -> None:
-    """Wipe public schema, then load from .sql. Raises on subprocess
-    failure; partial-restore state is the caller's concern."""
+    """Wipe public schema, then load from the custom-format dump. Raises on
+    subprocess failure; partial-restore state is the caller's concern."""
     libpq = _libpq_url(db_url)
     subprocess.run(
         [
@@ -194,8 +203,9 @@ def restore_db(*, db_url: str, sql_path: Path) -> None:
         ],
         capture_output=True, check=True, timeout=120,
     )
+    # Custom-format (-Fc) dumps are restored with pg_restore, not psql.
     subprocess.run(
-        ["psql", libpq, "-f", str(sql_path)],
+        ["pg_restore", "--no-owner", "--no-acl", "-d", libpq, str(sql_path)],
         capture_output=True, check=True,
         timeout=_DB_SUBPROCESS_TIMEOUT_S,
     )

tests/test_backup_service.py

@@ -69,7 +69,7 @@ def test_backup_db_writes_sql_and_manifest(tmp_path, fake_subprocess):
     )
     sql = Path(result["sql_path"])
     manifest = Path(result["manifest_path"])
-    assert sql.is_file() and sql.suffix == ".sql"
+    assert sql.is_file() and sql.suffix == ".dump"
     assert manifest.is_file() and manifest.suffix == ".json"
     assert result["kind"] == "db"
     assert result["tar_path"] is None
@@ -92,6 +92,12 @@ def test_backup_db_strips_sqlalchemy_psycopg_driver(tmp_path, fake_subprocess):
     assert "+psycopg" not in cmd[-1]
 
 
+def test_backup_db_uses_compressed_custom_format(tmp_path, fake_subprocess):
+    # -Fc → pg_restore-loadable, compressed dump (#739 backup polish).
+    backup_service.backup_db(db_url="postgresql://u@h/d", images_root=tmp_path)
+    assert "-Fc" in fake_subprocess[0]
+
+
 def test_backup_db_strips_asyncpg_driver(tmp_path, fake_subprocess):
     backup_service.backup_db(
         db_url="postgresql+asyncpg://u:p@h/d", images_root=tmp_path,
@@ -132,17 +138,19 @@ def test_backup_images_excludes_backups_and_quarantine(tmp_path, fake_subprocess
 
 
 def test_restore_db_drops_schema_then_loads(tmp_path, fake_subprocess):
-    sql_path = tmp_path / "fake.sql"
-    sql_path.write_text("SELECT 1;")
+    dump_path = tmp_path / "fake.dump"
+    dump_path.write_bytes(b"\x00fake custom dump")
     backup_service.restore_db(
-        db_url="postgresql://u@h/d", sql_path=sql_path,
+        db_url="postgresql://u@h/d", sql_path=dump_path,
     )
-    # Two psql calls: one with -c (DROP SCHEMA), one with -f (load).
+    # Two calls: psql -c (DROP SCHEMA), then pg_restore -d (load the dump).
     assert len(fake_subprocess) == 2
+    assert fake_subprocess[0][0] == "psql"
     assert "-c" in fake_subprocess[0]
     assert "DROP SCHEMA IF EXISTS public CASCADE" in fake_subprocess[0][-1]
-    assert "-f" in fake_subprocess[1]
-    assert str(sql_path) in fake_subprocess[1]
+    assert fake_subprocess[1][0] == "pg_restore"
+    assert "-d" in fake_subprocess[1]
+    assert str(dump_path) in fake_subprocess[1]
 
 
 # --- restore_images --------------------------------------------------

tests/test_tasks_backup.py

@@ -93,7 +93,7 @@ async def test_backup_db_task_creates_backup_run_row_status_ok(db_sync):
     ).one()
     assert row.kind == "db"
     assert row.status == "ok"
-    assert row.sql_path and row.sql_path.endswith(".sql")
+    assert row.sql_path and row.sql_path.endswith(".dump")
     assert row.size_bytes is not None and row.size_bytes > 0
     assert row.finished_at is not None
     assert row.error is None