fix: native_upgrade_converge.sh stops its cluster on exit (#1321)

CHANGELOG.md

@@ -320,6 +320,13 @@ true until the next version shipped.
 
 ### Fixed
 
+- `test/native_upgrade_converge.sh` no longer leaves its PostgreSQL server
+  running and its workdir in `/tmp` after every run (#1321). It armed its own
+  `trap cleanup EXIT` after `pgc_setup`, which replaced the harness's
+  `pgc_on_exit`; its cleanup now calls `pgc_on_exit` first. Selftest part 620
+  fails any suite that calls `pgc_setup` and arms an EXIT trap reaching neither
+  `pgc_on_exit` nor `pgc_teardown`. Reported by @OffgridwithJD.
+
 - An `ALTER TABLE` on a partitioned columnar table that does not recurse no
   longer opens the other partitions without a lock. `DETACH PARTITION`,
   `DETACH PARTITION ... CONCURRENTLY`, `ATTACH PARTITION` and `ALTER TABLE ONLY`

test/check_ledger.tsv

@@ -1642,6 +1642,11 @@ harness_selftest	610-a-declined-capability-is-not-silent	the verdict: names lost
 harness_selftest	610-a-declined-capability-is-not-silent	the verdict: names lost beside a skip naming the capability are declined	15;16;17;18;19	2026-10-12	pgc_capability_verdict has no declined outcome, so a lost name beside a skip reads silent
 harness_selftest	610-a-declined-capability-is-not-silent	the verdict: names lost with neither a failure nor a skip are silent	15;16;17;18;19	2026-10-12	pgc_capability_verdict answers declined for every input
 harness_selftest	610-a-declined-capability-is-not-silent	the verdict: no name lost is preserved	15;16;17;18;19	2026-10-12	pgc_capability_verdict answers declined for every input
+harness_selftest	620-a-suite-s-own-trap-must-stop	and is not satisfied by the name in a comment or in the next function	15;16;17;18;19	2026-10-13	the trap detector never reports a suite;the trap detector reads a one-line handler on into the next function;the trap detector reads comments as code
+harness_selftest	620-a-suite-s-own-trap-must-stop	and passes a handler that chains either, and a suite without pgc_setup	15;16;17;18;19	2026-10-13	the trap detector accepts only pgc_on_exit, not pgc_teardown
+harness_selftest	620-a-suite-s-own-trap-must-stop	no suite's own EXIT trap leaves its cluster running	15;16;17;18;19	2026-10-13	native_upgrade_converge.sh's cleanup without pgc_on_exit, so its EXIT trap replaces lib.sh's;the trap detector accepts only pgc_on_exit, not pgc_teardown
+harness_selftest	620-a-suite-s-own-trap-must-stop	premise: pgc_setup still arms the trap this part protects	15;16;17;18;19	never	-
+harness_selftest	620-a-suite-s-own-trap-must-stop	the detector flags a handler that reaches neither pgc_on_exit nor pgc_teardown	15;16;17;18;19	2026-10-13	the trap detector never reports a suite;the trap detector sees EXIT only at the end of a trap line
 index_am_support	index_am_support	btree answers its operator with the same rows the scan returns	15;16;17;18;19	2026-09-23	pgcolumnar_index_build_range_scan drops two rows in three, so the index misses entries
 index_am_support	index_am_support	btree builds an index over a columnar table	15;16;17;18;19	2026-09-23	pgcolumnar_index_build_range_scan errors, so CREATE INDEX fails
 index_am_support	index_am_support	gist answers its operator with the same rows the scan returns	15;16;17;18;19	2026-09-23	pgcolumnar_index_build_range_scan drops two rows in three, so the index misses entries

test/check_ledger_budget.txt

@@ -1443,4 +1443,18 @@ suites_not_covered 247
 #     never         1698 +  2 = 1700
 #     dated          361 +  8 =  369
 #     1700 + 369 == 2069
-checks_never_observed_red 1700
+#
+# #1321, a suite's own EXIT trap must still stop its cluster (OffgridwithJD):
+# 1700 -> 1701. Selftest part 620 enters with 5 checks from a five-major
+# harness_selftest run (1228/1228 on each of 15-19). Four arms are dated: the
+# population arm by the run before the fix (native_upgrade_converge.sh's cleanup
+# without pgc_on_exit), the three detector arms by five mutations of the
+# detector. The fifth, 'premise: pgc_setup still arms the trap this part
+# protects', is a fact about lib.sh, the fixture this part reads, and arrives
+# never.
+#
+#     ledger rows   2069 + 5 = 2074
+#     never         1700 + 1 = 1701
+#     dated          369 + 4 =  373
+#     1701 + 373 == 2074
+checks_never_observed_red 1701

test/native_upgrade_converge.sh

@@ -137,7 +137,12 @@ for v in "${_NUC_FIXTURES[@]}"; do
 	cp "$src" "$dst"
 	STAGED+=("$dst")
 done
+# CHAINED, not replacing: pgc_setup armed `trap pgc_on_exit EXIT`, and bash keeps
+# one EXIT trap. Without this line the trap below replaced it, and every run left
+# its server listening and its workdir in /tmp (selftest part 620). First, so it
+# reads the suite's exit status before anything here overwrites `$?`.
 cleanup() {
+	pgc_on_exit
 	for f in "${STAGED[@]:-}"; do
 		[ -n "$f" ] || continue
 		if [ -f "$f.pgcbak" ]; then

test/pytest/TESTS.md

@@ -144,6 +144,7 @@ behaviour, the source of that number is named.
 - [96. test_autovacuum_expire.py: the daemon expires retention tables only when told to](#96-test_autovacuum_expirepy-the-daemon-expires-retention-tables-only-when-told-to)
 - [97. test_rollup.py: incremental rollups fold appended row groups, and equal a full recompute](#97-test_rolluppy-incremental-rollups-fold-appended-row-groups-and-equal-a-full-recompute)
 - [98. test_timeseries_recipes.py: the gap-fill, LOCF and interpolation recipes answer what they claim](#98-test_timeseries_recipespy-the-gap-fill-locf-and-interpolation-recipes-answer-what-they-claim)
+- [99. test_suite_exit_trap.py: a suite's own EXIT trap must still stop its cluster](#99-test_suite_exit_trappy-a-suites-own-exit-trap-must-still-stop-its-cluster)
 
 ## 1. How to read a test in here
 
@@ -6639,3 +6640,20 @@ Red first: on main the how-to has no recipes, so the premise reads 0 and the com
 | --- | --- |
 | `test_the_recipes_are_in_the_how_to` | each of the three headings has a `sql` block naming `readings` |
 | `test_each_recipe_matches_the_reference` | the table is columnar; the gap fill, LOCF and interpolation match the Python reference for all 24 hours; the gap fill gives the same buckets in an Asia/Kolkata session; its aggregate runs as the vectorized grouped node when that is on |
+
+## 99. test_suite_exit_trap.py: a suite's own EXIT trap must still stop its cluster
+
+The pytest twin of selftest part 620 (#1321). `pgc_setup` arms `trap pgc_on_exit EXIT`, which stops the server, removes the workdir and gives a suite that died its accounting (#1233). Bash keeps one EXIT trap, so a suite that arms its own after `pgc_setup` must call back into `pgc_on_exit` or `pgc_teardown`. `native_upgrade_converge.sh` did not, and every run left its server listening and its workdir in `/tmp`.
+
+Independent of the part: it reads the suites and `lib.sh` as text but never sources or runs them, and its reader is Python's own. Trap lines are split with `shlex`. Comments are removed by the shell's rule (a `#` that starts a word, outside quotes, with backslash escapes) rather than by cutting everything after `#`. A one-line handler is read as that line, and any other up to its closing brace. The check names match the part's, five of them. It is not graded by `compare_to_bash`, which pairs `test/<stem>.sh` with `test_<stem>.py` and so cannot see a selftest part; `test_capability_decline.py` (parts 600 and 610) is the precedent.
+
+Written red first: on the tree before the fix, the population arm fails naming `native_upgrade_converge.sh` and nothing else. Five mutations of this file's reader each redden the same arms as the matching mutation of the part: it never reports, it keeps comments, it reads past a one-line handler, it accepts only `pgc_on_exit`, and it sees `EXIT` only last on the line.
+
+### Every test
+
+| test | what it holds |
+| --- | --- |
+| `test_the_detector_flags_a_handler_that_reaches_neither` | two suites the test writes, one trapping `EXIT` and one `EXIT INT TERM`, are both reported |
+| `test_the_detector_is_not_satisfied_by_a_comment_or_the_next_function` | the teardown named only in a comment, or only in the function after a one-line handler, does not count |
+| `test_the_detector_passes_a_handler_that_chains` | a handler calling `pgc_on_exit`, one calling `pgc_teardown`, a quoted handler, and a suite without `pgc_setup` are not reported |
+| `test_no_suites_own_exit_trap_leaves_its_cluster_running` | `pgc_setup` still arms `trap pgc_on_exit EXIT` (premise), and no suite in `test/` is reported |

test/pytest/expected_tests.txt

@@ -269,7 +269,9 @@
 # (#1286 review), collected over the NO_CLUSTER files on this tree.
 # guard_tests 413 -> 414 for #1305's test_an_unguarded_import_is_refused_under_xdist_too, by collection.
 # guard_tests 414 -> 415 for test_docs_how_to_sections.py (the lost pg_partman heading), by collection.
-guard_tests 415
+# guard_tests 415 -> 419 for #1321's test_suite_exit_trap.py, four tests, the twin of
+# selftest part 620, collected over the NO_CLUSTER files on this tree.
+guard_tests 419
 
 # The complement: tests that need the driver and a throwaway cluster. Until #1016 these ran
 # in no CI job at all -- a quarter of the corpus, green when somebody ran them by hand and

test/pytest/test_harness_deps.py

@@ -77,6 +77,9 @@
     # #1215: inner pytest runs of expect.declines and require_module. Needs no
     # cluster and no driver; it never reaches into the shell harness.
     "test_capability_decline.py",
+    # #1321: reads the suites and lib.sh as text, with its own Python reader. Needs
+    # no cluster and no driver, and never runs the shell harness.
+    "test_suite_exit_trap.py",
     # The classifier's own controls, split out of this file so the job whose file
     # list IS this list actually runs them. This file cannot be in the list: it
     # hands cluster-bound file names to pytest, so it needs what they need.
@@ -1199,6 +1202,11 @@ def test_the_job_installs_no_database_driver(expect):
         "matches on cannot become a hand-maintained copy that rots apart from it "
         "(#1040, where five of eight helpers had been unreadable since the pattern "
         "shipped)",
+    "test_suite_exit_trap.py":
+        "reads every suite's text and the helper library's setup function as DATA, "
+        "with its own Python reader, to find an EXIT trap that replaces the one setup "
+        "arms (#1321). It never sources or executes either: the rule is about what "
+        "the shell text says, so the text is the only place to read it",
 }
 
 _SHELL_NAMES = re.compile(

test/pytest/test_suite_exit_trap.py

@@ -0,0 +1,144 @@
+"""A suite's own EXIT trap must still stop its cluster (#1321).
+
+The pytest half of selftest part 620, with its own reader. It does not source, call
+or import lib.sh, the part, or any shell helper: it reads the suites as text, the
+way the part does, but through Python's shlex rather than sed and awk, so the twins
+share the rule and not the parser.
+
+pgc_setup arms `trap pgc_on_exit EXIT`; pgc_on_exit stops the server and removes the
+workdir (pgc_teardown) and gives a suite that died its accounting (#1233). Bash keeps
+one EXIT trap, so a suite that arms its own after pgc_setup must call back into one
+of the two. native_upgrade_converge.sh did not, and every run left its server
+listening and its workdir in /tmp.
+
+Needs no cluster and no driver.
+"""
+
+import pathlib
+import re
+import shlex
+
+HERE = pathlib.Path(__file__).resolve().parent
+TESTDIR = HERE.parent
+CHAIN = ("pgc_on_exit", "pgc_teardown")
+
+
+def _code(text):
+    """The text with shell comments removed: a `#` that starts a word, outside quotes.
+
+    Not "everything after #": `$#` and `${x#y}` are not comments. A backslash escapes
+    the next character except inside single quotes, as in the shell.
+    """
+    out = []
+    for line in text.splitlines():
+        cut, quote, i = None, None, 0
+        while i < len(line):
+            ch = line[i]
+            if ch == "\\" and quote != "'":
+                i += 2
+                continue
+            if quote:
+                if ch == quote:
+                    quote = None
+            elif ch in "'\"":
+                quote = ch
+            elif ch == "#" and (i == 0 or line[i - 1] in " \t;"):
+                cut = i
+                break
+            i += 1
+        out.append(line if cut is None else line[:cut])
+    return out
+
+
+def _handlers(lines):
+    """The handler of every `trap HANDLER ... EXIT ...` line."""
+    found = []
+    for line in lines:
+        try:
+            words = shlex.split(line)
+        except ValueError:
+            continue
+        if len(words) >= 3 and words[0] == "trap" and "EXIT" in words[2:]:
+            found.append(words[1])
+    return found
+
+
+def _body(lines, name):
+    """The function's own lines: one for `name() { ...; }`, else up to its `}`."""
+    for i, line in enumerate(lines):
+        if not line.startswith(name + "()"):
+            continue
+        if re.search(r"\{.*\}\s*$", line):
+            return [line]
+        body = [line]
+        for nxt in lines[i + 1:]:
+            body.append(nxt)
+            if nxt.startswith("}"):
+                break
+        return body
+    return []
+
+
+def _offenders(paths):
+    bad = []
+    for path in paths:
+        lines = _code(pathlib.Path(path).read_text())
+        if not any(re.match(r"\s*pgc_setup(\s|$)", ln) for ln in lines):
+            continue
+        for h in _handlers(lines):
+            if any(c in h for c in CHAIN):
+                continue
+            if any(c in ln for ln in _body(lines, h) for c in CHAIN):
+                continue
+            bad.append(pathlib.Path(path).name)
+            break
+    return bad
+
+
+SUITES = {
+    "replaces": "pgc_setup x\ncleanup() {\n\trm -f y\n}\ntrap cleanup EXIT\n",
+    "replaces_on_signals": "pgc_setup x\ncleanup() {\n\trm -f y\n}\ntrap cleanup EXIT INT TERM\n",
+    "names_it_in_a_comment": "pgc_setup x\ncleanup() {\n\trm -f y # then pgc_teardown\n}\ntrap cleanup EXIT\n",
+    "next_function_has_it": "pgc_setup x\nt() { a; }\nother() {\n\tpgc_teardown\n}\ntrap t EXIT\n",
+    "chains_the_wrapper": "pgc_setup x\ncleanup() {\n\tpgc_on_exit\n\trm -f y\n}\ntrap cleanup EXIT\n",
+    "chains_the_teardown": "pgc_setup x\nt() { a; pgc_teardown; }\ntrap t EXIT INT TERM\n",
+    "chains_in_quotes": "pgc_setup x\ntrap 'restore; pgc_on_exit' EXIT\n",
+    "no_setup": "cleanup() {\n\trm -f y\n}\ntrap cleanup EXIT\n",
+}
+
+
+def _built(tmp_path, *names):
+    paths = []
+    for n in names:
+        p = tmp_path / f"{n}.sh"
+        p.write_text(SUITES[n])
+        paths.append(p)
+    return paths
+
+
+def test_the_detector_flags_a_handler_that_reaches_neither(tmp_path, expect):
+    expect.text("\n".join(_offenders(_built(tmp_path, "replaces", "replaces_on_signals"))),
+                "replaces.sh\nreplaces_on_signals.sh",
+                "the detector flags a handler that reaches neither pgc_on_exit nor pgc_teardown")
+
+
+def test_the_detector_is_not_satisfied_by_a_comment_or_the_next_function(tmp_path, expect):
+    expect.text("\n".join(_offenders(_built(tmp_path, "names_it_in_a_comment", "next_function_has_it"))),
+                "names_it_in_a_comment.sh\nnext_function_has_it.sh",
+                "and is not satisfied by the name in a comment or in the next function")
+
+
+def test_the_detector_passes_a_handler_that_chains(tmp_path, expect):
+    got = _offenders(_built(tmp_path, "chains_the_wrapper", "chains_the_teardown",
+                            "chains_in_quotes", "no_setup"))
+    expect.text(f"[{' '.join(got)}]", "[]",
+                "and passes a handler that chains either, and a suite without pgc_setup")
+
+
+def test_no_suites_own_exit_trap_leaves_its_cluster_running(expect):
+    lib = _code((TESTDIR / "lib.sh").read_text())
+    setup = _body(lib, "pgc_setup")
+    expect.num(sum(1 for ln in setup if re.search(r"\btrap pgc_on_exit EXIT\b", ln)), 1,
+               "premise: pgc_setup still arms the trap this part protects")
+    got = _offenders(sorted(TESTDIR.glob("*.sh")))
+    expect.text(f"[{' '.join(got)}]", "[]", "no suite's own EXIT trap leaves its cluster running")

test/selftest/620-a-suite-s-own-trap-must-stop.sh

@@ -0,0 +1,68 @@
+# ---- a suite's own EXIT trap must still stop its cluster ---------------------
+#
+# pgc_setup arms `trap pgc_on_exit EXIT`, and pgc_on_exit is what stops the
+# server and removes the workdir (pgc_teardown) and gives a suite that died its
+# accounting (#1233). Bash keeps ONE EXIT trap, so a suite that arms its own
+# after pgc_setup replaces all of that unless its handler calls back into it.
+# objstore_module.sh found this first and chains; native_upgrade_converge.sh did
+# not: its `trap cleanup EXIT` restored the staged fixtures and nothing else, so
+# every run left a PostgreSQL server listening and its workdir in /tmp. Measured
+# on PG17: each of five converge runs left exactly one server, started within a
+# second of the run's last log line.
+#
+# READ FROM THE SUITES' OWN TEXT, the one place the rule can be checked for all
+# of them at once; driving each suite to see whether its server outlives it would
+# take every suite's full run. The detector is driven below on suites written for
+# it, so a detector that flags nothing cannot pass the population arm. Captured
+# text reaches grep and awk as here-strings, never through a pipe (part 080).
+
+_te_offenders() {
+	local f code h fn
+	for f in "$@"; do
+		code="$(sed 's/#.*//' "$f")"
+		grep -qE '^[[:space:]]*pgc_setup( |$)' <<<"$code" || continue
+		while IFS= read -r h; do
+			case "$h" in *pgc_on_exit*|*pgc_teardown*) continue ;; esac
+			# The handler's own body: one line for a one-line function, else up to
+			# its closing brace -- never on into the next function's.
+			fn="$(awk -v h="$h" '
+				!on && index($0, h "()") == 1 && $0 ~ /\{.*\}[[:space:]]*$/ { print; exit }
+				!on && index($0, h "()") == 1 { on = 1 }
+				on { print }
+				on && /^}/ { exit }' <<<"$code")"
+			grep -qE 'pgc_on_exit|pgc_teardown' <<<"$fn" && continue
+			basename "$f"
+			break
+		done < <(sed -nE "s/^[[:space:]]*trap[[:space:]]+(['\"]([^'\"]*)['\"]|([A-Za-z_][A-Za-z_0-9]*))[[:space:]]+(.*[[:space:]])?EXIT([[:space:]].*)?$/\2\3/p" <<<"$code")
+	done
+}
+
+check_num "premise: pgc_setup still arms the trap this part protects" \
+	"$(sed -n '/^pgc_setup()/,/^}/p' "$PGC_TESTDIR/lib.sh" | sed 's/#.*//' | grep -c 'trap pgc_on_exit EXIT')" "1"
+
+_te_tmp="$(mktemp -d)"
+_te_mk() { printf '%b\n' "$2" > "$_te_tmp/$1.sh"; }
+_te_mk replaces 'pgc_setup x\ncleanup() {\n\trm -f y\n}\ntrap cleanup EXIT'
+_te_mk replaces_on_signals 'pgc_setup x\ncleanup() {\n\trm -f y\n}\ntrap cleanup EXIT INT TERM'
+_te_mk names_it_in_a_comment 'pgc_setup x\ncleanup() {\n\trm -f y # then pgc_teardown\n}\ntrap cleanup EXIT'
+_te_mk next_function_has_it 'pgc_setup x\nt() { a; }\nother() {\n\tpgc_teardown\n}\ntrap t EXIT'
+_te_mk chains_the_wrapper 'pgc_setup x\ncleanup() {\n\tpgc_on_exit\n\trm -f y\n}\ntrap cleanup EXIT'
+_te_mk chains_the_teardown 'pgc_setup x\nt() { a; pgc_teardown; }\ntrap t EXIT INT TERM'
+_te_mk chains_in_quotes "pgc_setup x\ntrap 'restore; pgc_on_exit' EXIT"
+_te_mk no_setup 'cleanup() {\n\trm -f y\n}\ntrap cleanup EXIT'
+
+check_text "the detector flags a handler that reaches neither pgc_on_exit nor pgc_teardown" \
+	"$(_te_offenders "$_te_tmp/replaces.sh" "$_te_tmp/replaces_on_signals.sh")" \
+	"$(printf 'replaces.sh\nreplaces_on_signals.sh')"
+check_text "and is not satisfied by the name in a comment or in the next function" \
+	"$(_te_offenders "$_te_tmp/names_it_in_a_comment.sh" "$_te_tmp/next_function_has_it.sh")" \
+	"$(printf 'names_it_in_a_comment.sh\nnext_function_has_it.sh')"
+check_text "and passes a handler that chains either, and a suite without pgc_setup" \
+	"[$(_te_offenders "$_te_tmp/chains_the_wrapper.sh" "$_te_tmp/chains_the_teardown.sh" \
+		"$_te_tmp/chains_in_quotes.sh" "$_te_tmp/no_setup.sh")]" "[]"
+rm -rf "$_te_tmp"
+
+check_text "no suite's own EXIT trap leaves its cluster running" \
+	"[$(_te_offenders "$PGC_TESTDIR"/*.sh | tr '\n' ' ')]" "[]"
+
+unset -f _te_offenders _te_mk

test/selftest/parts.manifest

@@ -58,3 +58,4 @@
 590-a-declared-suite-must-account.sh
 600-a-missing-pyarrow-declines.sh
 610-a-declined-capability-is-not-silent.sh
+620-a-suite-s-own-trap-must-stop.sh