From b93a3bca16838d1fd9ae28d5008b7749f1b83736 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Mon, 29 Jun 2026 21:29:22 +0530 Subject: [PATCH] ci(postgres): fail setup if pg_ctl stop fails before baking the datadir MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Stop DB and stage datadir" step swallowed a failed `pg_ctl -m fast -w stop` with `|| true`, then moved and tarred the PGDATA regardless. A stop that times out or errors would bake a still-running, crash-inconsistent cluster into the artifact every test shard consumes — and with full_page_writes off, crash recovery can't repair torn pages. Drop the `|| true` so a failed stop fails the job, mirroring the MariaDB sister's "don't bake a dirty datadir" guard. Also drop the redundant `ALTER SYSTEM SET fsync/synchronous_commit/ full_page_writes = off` block from install.sh. Its comment claimed the postgres workflow "runs a service-container DB and never calls start-db.sh", but it does call start-db.sh, which already applies those flags via `-o` on every postgres start (setup job and each shard). The block was a no-op and its justification was factually wrong. Co-Authored-By: Claude Opus 4.8 --- .github/helper/install.sh | 12 ++++-------- .github/workflows/server-tests-postgres.yml | 6 +++++- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/helper/install.sh b/.github/helper/install.sh index 34e777506c9..27928ee8bbc 100644 --- a/.github/helper/install.sh +++ b/.github/helper/install.sh @@ -297,14 +297,10 @@ if [ "$DB" == "postgres" ];then echo "travis" | psql -h 127.0.0.1 -p 5432 -c "CREATE DATABASE test_frappe" -U postgres; echo "travis" | psql -h 127.0.0.1 -p 5432 -c "CREATE USER test_frappe WITH PASSWORD 'test_frappe'" -U postgres; - # Disposable CI DB: durability off for speed (postgres fsyncs every commit by default, which - # dominates a commit-heavy suite). All reloadable, no restart. The postgres workflow runs a - # service-container DB and never calls start-db.sh, so the flags must be applied here. - echo "travis" | psql -h 127.0.0.1 -p 5432 -U postgres \ - -c "ALTER SYSTEM SET synchronous_commit = 'off'" \ - -c "ALTER SYSTEM SET fsync = 'off'" \ - -c "ALTER SYSTEM SET full_page_writes = 'off'" \ - -c "SELECT pg_reload_conf()"; + # Durability-off for speed (no fsync/synchronous_commit/full_page_writes) is applied by + # start-db.sh's postgres `-o` flags on every start — setup job AND each test shard — so it is + # NOT repeated here. The postgres workflow runs in-runner via start-db.sh, not a service + # container. fi cd ~/frappe-bench || exit diff --git a/.github/workflows/server-tests-postgres.yml b/.github/workflows/server-tests-postgres.yml index 8cd50f235f0..98141162d09 100644 --- a/.github/workflows/server-tests-postgres.yml +++ b/.github/workflows/server-tests-postgres.yml @@ -108,7 +108,11 @@ jobs: - name: Stop DB and stage datadir run: | PG_BIN=$(ls -d /usr/lib/postgresql/*/bin | sort -V | tail -1) - "$PG_BIN/pg_ctl" -D /home/runner/pgdata -m fast -w stop || true + # Clean shutdown so the baked datadir is consistent. Do NOT swallow a failed stop with + # `|| true`: moving and tarring a still-running cluster ships a torn datadir the shards + # cannot crash-recover (full_page_writes is off). Fail the job instead — mirrors the + # MariaDB sister's "don't bake a dirty datadir" guard. + "$PG_BIN/pg_ctl" -D /home/runner/pgdata -m fast -w stop mv /home/runner/pgdata /home/runner/frappe-bench/pgdata - name: Package bench for test shards