Skip to content

fix(ansible): move pgBackRest spool-path off the 10GB root volume - #2312

Draft
hunleyd wants to merge 10 commits into
developfrom
hunleyd/indata-1038-pgbackrest-archive-async-spool-path-defaults-onto-the-10gb-root-volume
Draft

fix(ansible): move pgBackRest spool-path off the 10GB root volume#2312
hunleyd wants to merge 10 commits into
developfrom
hunleyd/indata-1038-pgbackrest-archive-async-spool-path-defaults-onto-the-10gb-root-volume

Conversation

@hunleyd

@hunleyd hunleyd commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • pgBackRest's spool-path (used by async archive-push/archive-get to queue transient data) was never set explicitly, so it defaulted to /var/spool/pgbackrest — which sits on the AMI's 10GB root volume, not /data (the dedicated EBS data volume PGDATA lives on).
  • Sets spool-path = /data/pgbackrest_spool in the ansible-managed [global] pgbackrest.conf, and updates the directory-creation task to create that path instead.
  • Updates the sbpostgres AppArmor profile (postgres_shell and pgbackrest_shell sub-profiles) to grant write access to /data/pgbackrest_spool instead of the old /var/spool/pgbackrest — the profile is loaded in enforce mode, so without this the config change alone would just trade "spool fills the root volume" for "every archive-push/archive-get write gets denied."
  • /data is the volume that survives pause/restore, pg_upgrade, resize-compute, and restore-to-new-project — any event that can throw away the existing EC2 instance and its root volume. The root volume does not survive those events, so anything written there is lost/reset along with it.
  • This is a [global]-scope pgBackRest option, set once at provisioning — no supabase-admin-agent changes needed. Split out of feat(sudoers): let supabase-admin-agent invoke pgbackrest reconcile as root #2291 (sudoers grant for pgbackrest reconcile) since the two are unrelated concerns: one is a privilege-escalation grant, this one is a storage-location fix, and neither depends on the other.

Why this is safe

  • pgBackRest's own documentation recommends spool-path live on the same filesystem as pg_wal, so archive-get can hand a WAL segment to Postgres with a rename instead of a copy. /data/pgdata/pg_wal is on /data, so this change satisfies that recommendation directly — this is the primary justification.
    • Secondary: the old /var/spool/pgbackrest also shared the 10GB root volume with the OS, systemd, and journal, so headroom there was already thin; moving off it removes one more consumer of that shared space. This trades that exposure for the spool queue now sharing /data with PGDATA instead — a smaller volume of contention (pgBackRest's archive-get-queue-max defaults to a bounded 128MiB), but worth naming as the tradeoff being accepted rather than a pure win.
  • Spool-path data is disposable.
    • Per pgBackRest docs, spool-path contents aren't durable state — archive-push rechecks each WAL segment against the repo and archive-get rebuilds its queue on loss. No migration needed for spool contents.
  • Matches the existing ownership pattern (pgbackrest:postgres) already applied to every other entry in the same directory-creation loop.

Known gap (tracked separately, not blocking this PR)

  • The directory-creation task above only runs at AMI-bake time. /data itself is root:root 0755, so an instance whose /data volume didn't come fresh from this AMI's own data snapshot (confirmed affected: resize-compute, hibernation-resume, pg major upgrade — see INDATA-1153) never gets /data/pgbackrest_spool created, and the unprivileged pgbackrest user can't self-create it there.
  • This is latent today because pgBackRest isn't fleet-deployed yet and archive-async is off everywhere (INDATA-996 is the fleet-wide enablement gate). INDATA-1153 needs to land before INDATA-996 ships fleet-wide, not before this PR merges.
  • The sbpostgres AppArmor profile duplicates its file-permission rules between the postgres_shell and pgbackrest_shell sub-profiles (pre-existing, not introduced by this PR) — tracked as INDATA-1180 for a future #include-based dedup.

Test plan

  • ansible-playbook --syntax-check on the full playbook — clean
  • Manual: confirm /data/pgbackrest_spool exists with pgbackrest:postgres ownership after provisioning, and archive-get/archive-push operate against it under the updated AppArmor profile

Resolves INDATA-1038

spool-path was never set explicitly, so pgBackRest defaulted to
/var/spool/pgbackrest, which lands on the AMI's root volume (10GB,
shared with the OS/systemd/journal) rather than the /data EBS volume
PGDATA lives on. archive-get's async replica-catch-up queue can hold
several 16MB+ WAL segments at once; once archive-async is enabled
fleet-wide (INDATA-996) that risks filling the root volume and taking
the instance down. Points spool-path at /data/pgbackrest_spool instead,
and updates the directory-creation task to match. Global option, set
once at provisioning — no supabase-admin-agent changes needed.
@hunleyd hunleyd self-assigned this Jul 23, 2026
…est.yml

The pgbackrest spool directory path is now split across two files
(pgbackrest.conf's spool-path and this task's dir-creation loop); add a
why-comment so a future edit to one doesn't silently drift from the other.
@blacksmith-sh

This comment has been minimized.

…nces

The comment explaining the /data spool-path override packed the AMI
root-volume constraint and the dedicated-EBS-volume fact into one
long clause. Split into two short sentences on the same line per
ASD-STE100 style rules.
@brainrake

Copy link
Copy Markdown
Collaborator

Looks good. Will need to make sure it migrates correctly on existing instances. I think we need to move the directory if it exists. It depends on how/when pg_backrest is run and what is in the directory.

hunleyd added 7 commits August 7, 2026 22:34
…c-spool-path-defaults-onto-the-10gb-root-volume

* origin/develop: (26 commits)
  Lots of bootstrap script clean ups (#2326)
  ci: Fix nix-build PUSH_TO_CACHE (#2345)
  ci/nix-install-ephemeral: Drop sticky disk config (#2346)
  chore: bump postgres_release to cut fresh AMIs (includes #2334) (#2349)
  fix(multigres): stop base config data_directory from overriding pooler data dir (#2344)
  fix(cron): remove unnecessary TRIGGER grant on cron.job_run_details from postgres (#2334)
  chore: enable extension version restriction (warn) and cut AMIs (#2315)
  feat(nix): add site-env packages for rolling instance updates (#2283)
  ci: resolve package install sha from a flake input override (#2327)
  fix(multigres): remove dangling wal-g include from postgresql.conf (#2338)
  fix: Allow test CI pipelines to use old Int CA access. For staging only (#2330)
  chore(nix): remove maintainers field from package definitions (#2280)
  chore: bump multigres to b713432 (#2323)
  ci: Use arm-native-runner for kvm builds on aarch64-linux (#2319)
  Whole lot of ansible clean up (#2272)
  fix(ansible): drop no-op zpool loop item from zswap task (#2322)
  ci: Fix dockerhub-release-matrix matrix generation (#2320)
  chore: bump pgctld (#2318)
  fix(ansible): skip zswap params the kernel does not expose (#2321)
  feat(docker): add Dockerfile-supabase base image and rewrite Dockerfile-multigres as layered image (#2160)
  ...
sbpostgres_apparmor's postgres_shell and pgbackrest_shell sub-profiles
still only allowed writes to /var/spool/pgbackrest, the path this PR
moved off of. Once archive-async is enabled, pgbackrest's writes to
/data/pgbackrest_spool would be denied under AppArmor enforce mode,
turning this fix into a new archive-push/archive-get failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants