Check for 2 GB free space before PG upgrade#2271
Conversation
initiate.sh realizes the new pg_upgrade Nix store path onto / before touching the mounted data disk. If / is already tight on space, Nix fills the partition mid-upgrade and the failure surfaces deep into the run instead of at the start. check_free_space in common.sh checks available space on / via df and aborts immediately, before any mkdir/nix work, if less than 2 GiB is free.
df -k wraps onto a second line when the source device name is long, which misaligns the NR==2 awk parse; -P forces single-line POSIX output. Also validate available_kb is numeric before comparing — an empty/non-numeric value from a bad df parse made the bash [ -lt ] test error inside an if condition, which set -e doesn't catch, silently skipping the guard instead of aborting.
[ -lt ] swapped for (( < )) per review — script is bash, not posix sh, so arithmetic mode reads clearer.
…ree-space-before-pg-upgrade
treefmt's shfmt formatting normalizes `(( expr ))` to `((expr))` with no inner padding; the added check_free_space guard used the padded form, failing pre-commit-run/treefmt-check CI on all platforms.
check_free_space only ran once, at the top of initiate_upgrade, but the tarball extraction, Nix install, and catalog download that follow can eat into that 2 GiB reserve before nix-store -r (the actual big consumer) runs, letting a host that passed the entry check still hit ENOSPC mid-realize. Add a second 2 GiB check immediately before the nix-store -r retry loop to confirm the reserve is still intact right where it's needed, and a smaller 512 MiB check after the closure is realized, since the remaining apt-get/ locale-gen work needs far less headroom than the Nix realize did.
|
@hunleyd do we still want to merge this in lieu of the linear update? Will leave it to you to merge if so or close otherwise. |
… running check_free_space runs before $MOUNT_POINT is ever mounted, so when it fails the ERR trap's cleanup() hits an umount on an unmounted path. Every other risky command in cleanup() already has || true; this one didn't, so under set -e the trap handler aborted before writing UPGRADE_STATUS, leaving /tmp/pg-upgrade-status stuck at "running" forever on exactly the low-disk failure path this PR adds. Reproduced and confirmed fixed with a minimal harness matching the trap structure (removed after verifying).
|
@mmlb yes, still want this merged. Dug into it: admin-api's `getFreeDiskSpace` only gates the download step (requires free space ≥ 2x the object being downloaded), not the actual upgrade — the failure mode this PR targets is `nix-store` realizing new derivations onto `/` during `pg_upgrade`, well after the download completes. Different check, different point in the pipeline, no overlap. The Linear closure on INDATA-641 was specifically about not vendoring a copy of these scripts into Salt (supabase/salt#685) — this PR (the actual source of truth) still stands. Also pushed a fix: `cleanup()`'s `retry 3 umount $MOUNT_POINT` had no `|| true`, so when `check_free_space` fails before the disk is ever mounted, the ERR trap's own umount call was aborting under set -e before writing the status file — leaving it stuck at "running" forever on exactly the failure path this PR adds. Fixed and reproduced/verified with a minimal harness. Enabling auto-merge now that CI's running clean. |
2 participants
Summary
initiate.shrealizes the new pg_upgrade Nix store path onto/before touching the mounted data disk, so a tight root partition fills mid-upgrade instead of failing fast at the start.check_free_spacetocommon.sh: checks available space on/viadfand returns non-zero if below a threshold.initiate_upgrade()ininitiate.shwith a 2 GiB threshold, before any mkdir/nix work. A failure there hits the existingtrap cleanup ERRpath, same as every other failure in this script — postgresql restarts and status is set tofailed.Resolves INDATA-641
Test plan
bash -nsyntax check on both modified scriptscheck_free_spacewith a trivial threshold (pass) and an absurd threshold (fail) against realdfoutputtests/pg_upgrade)