On Wed, Sep 03, 2026 at 11:04:39AM +0900, Michael Paquier wrote: > On Wed, Sep 02, 2026 at 06:13:26PM -0700, Bharath Rupireddy wrote: >> Nice! Do you mind adding the reproducer as a TAP test for HEAD? > > I'm assuming that it should be possible to use an injection points > based on the fact that we would be up and running for the inserts.
Thanks, both. Attached is v2: 0001 is the fix, unchanged from v1, and 0002 adds the reproducer as a TAP test on HEAD. The test adds a small module, src/test/modules/test_walwriter, with one C function that requests a segment switch and then stores the resulting insert position in asyncXactLSN. After a switch that position is just past the new segment's long page header, beyond the end of generated WAL, so the walwriter's next cycle requests a flush past the end of generated WAL -- the same shape as the production request. I did look at injection points first, but there is no INJECTION_POINT() in this path, and what the reproducer needs is a bogus value stored into asyncXactLSN rather than a backend stopped at a particular point, which would require a custom callback and hence a test module anyway. A plain test module also keeps the test runnable in builds without injection point support. The timing side needs no help: with the test's wal_writer_flush_after = 0, XLogSetAsyncXactLSN() wakes the walwriter, which picks the value up on its next cycle, so the test just waits for the existing "request to flush past end of generated WAL" message to show up in the log. After that message, the test checks that the advertised flush position is still below the bogus request, that no child process was terminated, and that normal WAL activity afterwards gets past that position. On unpatched HEAD the test fails in both assert and production builds: XLogWrite() hits its "xlog write request ... is past end of log" PANIC and the walwriter's crash takes the server down (TAP clusters run with restart_after_crash = off). (Which sanity check fires first depends on the WAL buffer state; with 1MB segments I had seen the Insert >= Write assertion instead.) With 0001 applied, both build types pass the test. Regards, Paul
From c24a51d0ad2e01681cdaa7781dc016c18a7c0184 Mon Sep 17 00:00:00 2001 From: Paul Kim <[email protected]> Date: Wed, 2 Sep 2026 14:40:25 +0900 Subject: [PATCH v2 1/2] Honor WAL insertion clamp in XLogBackgroundFlush WaitXLogInsertionsToFinish() clamps a request that is past the end of reserved WAL and returns the safe position. XLogBackgroundFlush() ignored that return value and passed its original request to XLogWrite(). In a non-assert build, a bogus asyncXactLSN just after a segment boundary can consequently advance the advertised write and flush positions through the new page header. A walsender can send that header alone, after which a standby can interpret stale contents of a recycled segment as a record. Assert builds instead fail the Insert >= Write invariant. Use the returned position when it is smaller than the request, and clamp both the write and flush targets. Do not assign it unconditionally, because the normal return value can be beyond the requested position. Also update the header comment of WaitXLogInsertionsToFinish(), which claimed that the return value is always >= 'upto', contradicting the clamp documented in the function body. --- src/backend/access/transam/xlog.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index de4c96e135f..a81b0522663 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1541,7 +1541,10 @@ WALInsertLockUpdateInsertingAt(XLogRecPtr insertingAt) * Returns the location of the oldest insertion that is still in-progress. * Any WAL prior to that point has been fully copied into WAL buffers, and * can be flushed out to disk. Because this waits for any insertions older - * than 'upto' to finish, the return value is always >= 'upto'. + * than 'upto' to finish, the return value is normally >= 'upto'. However, + * if 'upto' is past the end of reserved WAL, the request is clamped to the + * current reserved position, and the return value can be smaller than + * 'upto'. Callers must not write or flush past the returned position. * * Note: When you are about to write out WAL, you must call this function * *before* acquiring WALWriteLock, to avoid deadlocks. This function might @@ -3011,6 +3014,7 @@ bool XLogBackgroundFlush(void) { XLogwrtRqst WriteRqst; + XLogRecPtr insertpos; bool flexible = true; static TimestampTz lastflush; TimestampTz now; @@ -3114,8 +3118,18 @@ XLogBackgroundFlush(void) START_CRIT_SECTION(); - /* now wait for any in-progress insertions to finish and get write lock */ - WaitXLogInsertionsToFinish(WriteRqst.Write); + /* now wait for any in-progress insertions to finish */ + insertpos = WaitXLogInsertionsToFinish(WriteRqst.Write); + + /* honor the clamp if the request was past the end of reserved WAL */ + if (insertpos < WriteRqst.Write) + { + WriteRqst.Write = insertpos; + if (WriteRqst.Flush > insertpos) + WriteRqst.Flush = insertpos; + } + + /* get write lock */ LWLockAcquire(WALWriteLock, LW_EXCLUSIVE); RefreshXLogWriteResult(LogwrtResult); if (WriteRqst.Write > LogwrtResult.Write || -- 2.50.1 (Apple Git-155)
From 98fa70914199a44a0911172823b8cffad8e15871 Mon Sep 17 00:00:00 2001 From: Paul Kim <[email protected]> Date: Fri, 4 Sep 2026 08:29:54 +0900 Subject: [PATCH v2 2/2] Add a TAP test for the WAL insertion clamp in XLogBackgroundFlush The new test_walwriter module stores a position where nothing has been inserted in asyncXactLSN: it requests a segment switch and takes the resulting insert position, which lies past the new segment's long page header, beyond the end of generated WAL. The walwriter's next cycle then requests a flush past the end of generated WAL. Without the previous commit, the walwriter fails on the "xlog write request is past end of log" check in XLogWrite() and its crash takes the server down, in both assert and production builds. With it, the request is clamped, the existing diagnostic LOG message is emitted, and normal WAL activity proceeds afterwards. --- src/test/modules/Makefile | 1 + src/test/modules/meson.build | 1 + src/test/modules/test_walwriter/.gitignore | 4 ++ src/test/modules/test_walwriter/Makefile | 23 +++++++ src/test/modules/test_walwriter/meson.build | 33 ++++++++++ .../test_walwriter/t/001_bogus_async_lsn.pl | 64 +++++++++++++++++++ .../test_walwriter/test_walwriter--1.0.sql | 8 +++ .../modules/test_walwriter/test_walwriter.c | 46 +++++++++++++ .../test_walwriter/test_walwriter.control | 4 ++ 9 files changed, 184 insertions(+) create mode 100644 src/test/modules/test_walwriter/.gitignore create mode 100644 src/test/modules/test_walwriter/Makefile create mode 100644 src/test/modules/test_walwriter/meson.build create mode 100644 src/test/modules/test_walwriter/t/001_bogus_async_lsn.pl create mode 100644 src/test/modules/test_walwriter/test_walwriter--1.0.sql create mode 100644 src/test/modules/test_walwriter/test_walwriter.c create mode 100644 src/test/modules/test_walwriter/test_walwriter.control diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile index bb88b3058ed..ac949be4695 100644 --- a/src/test/modules/Makefile +++ b/src/test/modules/Makefile @@ -54,6 +54,7 @@ SUBDIRS = \ test_slru \ test_tidstore \ test_wait_lsn \ + test_walwriter \ unsafe_tests \ worker_spi \ xid_wraparound diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build index ce09e00531d..473d1a7bf87 100644 --- a/src/test/modules/meson.build +++ b/src/test/modules/meson.build @@ -55,6 +55,7 @@ subdir('test_shm_mq') subdir('test_slru') subdir('test_tidstore') subdir('test_wait_lsn') +subdir('test_walwriter') subdir('typcache') subdir('unsafe_tests') subdir('worker_spi') diff --git a/src/test/modules/test_walwriter/.gitignore b/src/test/modules/test_walwriter/.gitignore new file mode 100644 index 00000000000..5dcb3ff9723 --- /dev/null +++ b/src/test/modules/test_walwriter/.gitignore @@ -0,0 +1,4 @@ +# Generated subdirectories +/log/ +/results/ +/tmp_check/ diff --git a/src/test/modules/test_walwriter/Makefile b/src/test/modules/test_walwriter/Makefile new file mode 100644 index 00000000000..aa55d2cd8b5 --- /dev/null +++ b/src/test/modules/test_walwriter/Makefile @@ -0,0 +1,23 @@ +# src/test/modules/test_walwriter/Makefile + +MODULE_big = test_walwriter +OBJS = \ + $(WIN32RES) \ + test_walwriter.o +PGFILEDESC = "test_walwriter - test facilities for the WAL writer" + +EXTENSION = test_walwriter +DATA = test_walwriter--1.0.sql + +TAP_TESTS = 1 + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_walwriter +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/src/test/modules/test_walwriter/meson.build b/src/test/modules/test_walwriter/meson.build new file mode 100644 index 00000000000..9c2afc4cb6c --- /dev/null +++ b/src/test/modules/test_walwriter/meson.build @@ -0,0 +1,33 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +test_walwriter_sources = files( + 'test_walwriter.c', +) + +if host_system == 'windows' + test_walwriter_sources += rc_lib_gen.process(win32ver_rc, extra_args: [ + '--NAME', 'test_walwriter', + '--FILEDESC', 'test_walwriter - test facilities for the WAL writer',]) +endif + +test_walwriter = shared_module('test_walwriter', + test_walwriter_sources, + kwargs: pg_test_mod_args, +) +test_install_libs += test_walwriter + +test_install_data += files( + 'test_walwriter.control', + 'test_walwriter--1.0.sql', +) + +tests += { + 'name': 'test_walwriter', + 'sd': meson.current_source_dir(), + 'bd': meson.current_build_dir(), + 'tap': { + 'tests': [ + 't/001_bogus_async_lsn.pl', + ], + }, +} diff --git a/src/test/modules/test_walwriter/t/001_bogus_async_lsn.pl b/src/test/modules/test_walwriter/t/001_bogus_async_lsn.pl new file mode 100644 index 00000000000..ca0100dea37 --- /dev/null +++ b/src/test/modules/test_walwriter/t/001_bogus_async_lsn.pl @@ -0,0 +1,64 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +# Test that XLogBackgroundFlush() honors the clamp applied by +# WaitXLogInsertionsToFinish() when the flush request is past the end +# of generated WAL. Without the fix, the walwriter fails on one of +# XLogWrite()'s sanity checks (which one fires first depends on the +# WAL buffer state) and takes the server down, in both assert and +# production builds. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +my $node = PostgreSQL::Test::Cluster->new('primary'); +$node->init; +$node->append_conf( + 'postgresql.conf', qq( +autovacuum = off +wal_writer_delay = 10ms +wal_writer_flush_after = 0 +)); +$node->start; + +$node->safe_psql('postgres', 'CREATE EXTENSION test_walwriter'); + +# Create a table for the later INSERT; this also guarantees that the +# segment switch performed by the injection is not a no-op. +$node->safe_psql('postgres', 'CREATE TABLE t AS SELECT 1 AS i'); + +my $log_offset = -s $node->logfile; + +# Store a position where nothing has been inserted in asyncXactLSN. +my $injected = $node->safe_psql('postgres', + 'SELECT test_walwriter_bogus_async_lsn()'); + +# The walwriter's next cycle picks up the bogus request and logs the +# clamp. +$node->wait_for_log(qr/request to flush past end of generated WAL/, + $log_offset); + +# The advertised flush position must not include the bogus request. +my $result = $node->safe_psql('postgres', + qq{SELECT pg_current_wal_flush_lsn() < '$injected'::pg_lsn}); +is($result, 't', 'flush position stays below the bogus request'); + +# The walwriter must not have failed one of XLogWrite()'s sanity +# checks: no child process may have been terminated. +my $log = slurp_file($node->logfile, $log_offset); +unlike( + $log, + qr/terminating any other active server processes/, + 'no crash after the bogus flush request'); + +# Normal WAL activity gets past the bogus position. +$node->safe_psql('postgres', 'INSERT INTO t VALUES (2)'); +$node->safe_psql('postgres', 'SELECT pg_switch_wal()'); +$result = $node->safe_psql('postgres', + qq{SELECT pg_current_wal_flush_lsn() > '$injected'::pg_lsn}); +is($result, 't', 'flush position advances past the bogus request'); + +$node->stop; +done_testing(); diff --git a/src/test/modules/test_walwriter/test_walwriter--1.0.sql b/src/test/modules/test_walwriter/test_walwriter--1.0.sql new file mode 100644 index 00000000000..dd9c3e142ce --- /dev/null +++ b/src/test/modules/test_walwriter/test_walwriter--1.0.sql @@ -0,0 +1,8 @@ +/* src/test/modules/test_walwriter/test_walwriter--1.0.sql */ + +-- complain if script is sourced in psql, rather than via CREATE EXTENSION +\echo Use "CREATE EXTENSION test_walwriter" to load this file. \quit + +CREATE FUNCTION test_walwriter_bogus_async_lsn() +RETURNS pg_lsn +AS 'MODULE_PATHNAME' LANGUAGE C; diff --git a/src/test/modules/test_walwriter/test_walwriter.c b/src/test/modules/test_walwriter/test_walwriter.c new file mode 100644 index 00000000000..4360083fff0 --- /dev/null +++ b/src/test/modules/test_walwriter/test_walwriter.c @@ -0,0 +1,46 @@ +/*-------------------------------------------------------------------------- + * + * test_walwriter.c + * Test facilities for the WAL writer. + * + * Copyright (c) 2026, PostgreSQL Global Development Group + * + * IDENTIFICATION + * src/test/modules/test_walwriter/test_walwriter.c + * + * ------------------------------------------------------------------------- + */ +#include "postgres.h" + +#include "access/xlog.h" +#include "access/xlog_internal.h" +#include "fmgr.h" +#include "utils/pg_lsn.h" + +PG_MODULE_MAGIC; + +PG_FUNCTION_INFO_V1(test_walwriter_bogus_async_lsn); + +/* + * Store a position where nothing has been inserted in asyncXactLSN, and + * return it. + * + * Request a segment switch, then store the current insert position in + * asyncXactLSN. After the switch that position is just past the new + * segment's long page header, beyond the end of generated WAL, so the + * walwriter's next cycle requests a flush past the end of generated WAL. + * (If the insert position was already at a segment boundary the switch is + * a no-op, but the position returned still lies past the end of reserved + * WAL, so the outcome is the same.) + */ +Datum +test_walwriter_bogus_async_lsn(PG_FUNCTION_ARGS) +{ + XLogRecPtr ptr; + + (void) RequestXLogSwitch(false); + ptr = GetXLogInsertRecPtr(); + XLogSetAsyncXactLSN(ptr); + + PG_RETURN_LSN(ptr); +} diff --git a/src/test/modules/test_walwriter/test_walwriter.control b/src/test/modules/test_walwriter/test_walwriter.control new file mode 100644 index 00000000000..a196863447c --- /dev/null +++ b/src/test/modules/test_walwriter/test_walwriter.control @@ -0,0 +1,4 @@ +comment = 'Test code for the WAL writer' +default_version = '1.0' +module_pathname = '$libdir/test_walwriter' +relocatable = true -- 2.50.1 (Apple Git-155)
