From 3723d97ccbbd5b35844ea7e638ea6033f5e76115 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Mon, 26 Oct 2026 19:08:27 +0000 Subject: [PATCH] test: native_backend_crash's post-flush kill runs first, so a crashed write's leftover blocks cannot hide its write (#1757) The nightly on PG 17 (run 40376459928) failed one premise in the pytest twin: "crashB: the INSERT had written pages before the kill (the relation grew)", got False. The cause is the scenario order, not the product. A killed write's stripe reservation (the metapage highwater) is WAL-logged but not yet flushed, while its pages already extend the file. If the walwriter has not flushed it by the SIGSEGV, redo restores the old highwater over blocks that exist, and the next write fills those blocks before it extends anything. crashA kills a 3,000,000-row INSERT once the relation grows, and crashB's 100,000 rows then fitted entirely inside crashA's leftover. Reusing blocks that no durable reservation covers is correct, and every content check passed. Measured on a local PG 17 with crashA followed by a committed 100,000-row INSERT: - wal_writer_delay=10s: the relation did not grow, 3 runs of 3. - wal_writer_delay=200ms: it grew, 3 runs of 3. Both suites now: - hold the WAL writer off (wal_writer_delay = 10s, its maximum), so the race always goes the losing way instead of depending on runner load; - run crashB first, where it follows only the committed baseline. crashA and crashC write 3,000,000 rows, which outgrow any leftover. With the pin alone, on the old order, both are red on crashB's premise: the shell 27 passed + 1 failed, pytest 19 pass + 1 fail. With the reorder, both are green 5 runs of 5 (28 checks each). A flush before the kill (committing an xid from an observer) was tried and does not close it. The writer can reserve a further stripe between the flush and the kill; the shell went red once in 3. No check is added, renamed or redated, so the ledger and census are unchanged. Co-Authored-By: Claude Opus 5.5 --- test/native_backend_crash.sh | 19 +++++++++++++++---- test/pytest/test_native_backend_crash.py | 18 +++++++++++++----- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/test/native_backend_crash.sh b/test/native_backend_crash.sh index 07769259d..a68485549 100755 --- a/test/native_backend_crash.sh +++ b/test/native_backend_crash.sh @@ -29,6 +29,10 @@ pgc_setup "${1:-/usr/local/pg17/bin/pg_config}" # The postmaster must reinitialize a crashed backend rather than shut the cluster # down; that is the default, set here so the suite does not depend on it. psql_run "ALTER SYSTEM SET restart_after_crash = on;" +# THE WAL WRITER IS HELD OFF (its 10s maximum), as a busy runner can leave it, so +# a kill always loses the killed write's unflushed WAL. See the post-flush +# scenario below for why that decides the order of the scenarios. +psql_run "ALTER SYSTEM SET wal_writer_delay = '10s';" psql_run "SELECT pg_reload_conf();" >/dev/null # Heap mirror and columnar table, same committed baseline in both. @@ -187,14 +191,21 @@ assert_intact() { # label check "$1: the killed insert left no partial row group" "$(rowgroups)" "$base_groups" } -# --- kill while a large insert is mid-write ---------------------------------- -crash_one_backend 3000000 crashA insert -assert_intact "mid-write" - # --- kill after the stripes have flushed, before the commit that never comes - +# FIRST, because its write is the small one. A killed write's stripe reservation +# (the metapage highwater) is WAL-logged but unflushed, while its pages already +# extend the file. Redo then restores the old highwater over blocks that exist, +# and the next write fills them before it extends anything: after crashA, all +# 100000 rows of this one fit, the relation never grew, and its premise failed +# (nightly 40376459928, PG 17). Here it follows only the committed baseline. +# crashA and crashC write 3000000 rows, which outgrow any such leftover. crash_one_backend 100000 crashB sleep assert_intact "post-flush" +# --- kill while a large insert is mid-write ---------------------------------- +crash_one_backend 3000000 crashA insert +assert_intact "mid-write" + # --- kill with a concurrent reader holding a connection ---------------------- # The reinit drops every session, not only the one that died. This scenario adds # a second parked reader on top of the sentinel and asserts it too was dropped, diff --git a/test/pytest/test_native_backend_crash.py b/test/pytest/test_native_backend_crash.py index 1c7b8aed4..c8f8a474f 100644 --- a/test/pytest/test_native_backend_crash.py +++ b/test/pytest/test_native_backend_crash.py @@ -49,6 +49,9 @@ def nbc(pgc_cluster): c.execute("CREATE EXTENSION IF NOT EXISTS pgcolumnar") # the postmaster must reinitialize after a backend crash, not shut down c.execute("ALTER SYSTEM SET restart_after_crash = on") + # the WAL writer held off, as the shell does: a kill always loses the + # killed write's unflushed WAL, which is what orders the scenarios below + c.execute("ALTER SYSTEM SET wal_writer_delay = '10s'") c.execute("SELECT pg_reload_conf()") c.execute("CREATE TABLE h (id int, v text, x float8)") c.execute("CREATE TABLE c (id int, v text, x float8) USING pgcolumnar") @@ -213,16 +216,21 @@ def base(nbc): "mirror": _rows(nbc, "SELECT id, v, x FROM h"), "pm": _postmaster_pid(nbc)} -def test_a_kill_mid_write(nbc, base, expect): - dropped = _crash_one_backend(nbc, expect, 3000000, "crashA", "insert") - _assert_intact(nbc, expect, "mid-write", dropped, base) - - def test_a_kill_after_the_stripes_flushed(nbc, base, expect): + """FIRST, because its write is the small one. A killed write's stripe reservation + is WAL-logged but unflushed while its pages already extend the file, so redo + restores the old highwater over blocks that exist and the next write fills them + first: after crashA, all 100000 rows fit, the relation never grew, and the premise + failed (nightly 40376459928, PG 17). crashA and crashC outgrow any leftover.""" dropped = _crash_one_backend(nbc, expect, 100000, "crashB", "sleep") _assert_intact(nbc, expect, "post-flush", dropped, base) +def test_a_kill_mid_write(nbc, base, expect): + dropped = _crash_one_backend(nbc, expect, 3000000, "crashA", "insert") + _assert_intact(nbc, expect, "mid-write", dropped, base) + + def test_a_kill_with_a_second_reader(nbc, base, expect): """The reinit drops every session, so a second parked reader goes too.""" reader = _Bystander(nbc, 90)