public inbox for [email protected]  
help / color / mirror / Atom feed
From: Kirill Reshke <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Chao Li <[email protected]>
Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
Date: Thu, 18 Dec 2025 23:07:20 +0500
Message-ID: <CALdSSPgKH7TdW0zcDPjBxx+MX18bjc65K12KgoGtX6ZP8Qq63w@mail.gmail.com> (raw)
In-Reply-To: <CAAKRu_baqEkdJ0_1vokvgbaN52ycR65HM2-DR-VsRmYVVQLV8w@mail.gmail.com>
References: <CAAKRu_ZP-3=SaZykpwDBMJOdUKyW3Wm5JZfPFRR3L5Ac8ouq4w@mail.gmail.com>
	<CAAKRu_bgkOQqu3K5n4YLRsNBZqJ9Rjg80ROqgKSr2UGz4b5hUg@mail.gmail.com>
	<2wk7jo4m4qwh5sn33pfgerdjfujebbccsmmlownybddbh6nawl@mdyyqpqzxjek>
	<CAAKRu_YR-COJ9aGnMUQqt5yoWmUBjikqrd4jGNZYouHaXpis9g@mail.gmail.com>
	<CALdSSPhiCwJwWwgJP1NmqRmnp9RS2tGOBY0gQrfLCbB+OS5_KQ@mail.gmail.com>
	<CAAKRu_YS+Ocm=OzMaZnG4egFiE9v4VYfZ25DXd6jbwegqmGYbQ@mail.gmail.com>
	<CAAKRu_ZGZSqhGt-RcmmfiSheC+1fjQdxy6_+oM-1jMn8hyVptQ@mail.gmail.com>
	<CALdSSPg+B8RTzTXhJvCcKJBqgzhPZkq0E2oqxQdv74ZNZOMVzg@mail.gmail.com>
	<CAAKRu_Zha7HcdQBv8tTtQrcry5332J6kHnOc1X=TT03LzUXDow@mail.gmail.com>
	<CAAKRu_amF00f2T_H8N6pbbe75C22EeX1OqA=svpj8LNO1sdUuw@mail.gmail.com>
	<mhf4vkmh3j57zx7vuxp4jagtdzwhu3573pgfpmnjwqa6i6yj5y@sy4ymcdtdklo>
	<CAAKRu_agCLAQ9OjmrTdJe-X=Xr7QnU4d=cfxdQGwc9jNx9w31w@mail.gmail.com>
	<CAAKRu_ayWLg=WDGZZfSPWf0KjPM8u=LBb0D6XaEWyx2_YFFwAQ@mail.gmail.com>
	<CALdSSPhtPK36_sr9yFo3cN9TXQmjvSX3BZXCF8fVBLX-GETD0Q@mail.gmail.com>
	<CAAKRu_Yt6EH5aFSJBm-k7PrNM4bTt56fTRbyU7gqYXe4cW+F9g@mail.gmail.com>
	<CALdSSPhH7hi+EzYqq0=eMCthi7iNpY_YyECAC1qxPb7rd0TLrw@mail.gmail.com>
	<CAAKRu_baqEkdJ0_1vokvgbaN52ycR65HM2-DR-VsRmYVVQLV8w@mail.gmail.com>

On Thu, 18 Dec 2025 at 20:18, Melanie Plageman
<[email protected]> wrote:

> But you are right, I don't see any non-error code path where a heap
> page would become empty (all line pointers set unused) and then not be
> set all-visible. Only vacuum sets line pointers unused and if all the
> line pointers are unused it will always set the page all-visible.
>
> I think, though, that if we error out in lazy_scan_prune() after
> returning from heap_page_prune_and_freeze() such that we don't set the
> empty page all-visible, we can end up with an empty page without
> PD_ALL_VISIBLE set. You can see how this might work by patching the VM
> set code in lazy_scan_prune() to skip empty pages.
>

Thank you for your explanation!  I completely forgot that PD_ALL_VIS
is a non-persistent change (hint bit). so its update can be trivially
lost.
The simplest real-life example is being killed just after returning
from heap_page_prune_and_freeze, yes.
PFA tap test covering lazy_scan_new_or_empty code path for
empty-but-not-all-visible page

-- 
Best regards,
Kirill Reshke


Attachments:

  [application/octet-stream] v1-0001-Add-TAP-test-for-empty-page-vacuum.patch (4.3K, 2-v1-0001-Add-TAP-test-for-empty-page-vacuum.patch)
  download | inline diff:
From ac838953de9c4ab0cb5f13d1e1b8ad0a18e73e39 Mon Sep 17 00:00:00 2001
From: reshke <[email protected]>
Date: Thu, 18 Dec 2025 18:00:22 +0000
Subject: [PATCH v1] Add TAP test for empty page vacuum.

VACUUM can be run for empty pages with DISABLE_PAGE_SKIPPING option.
In this case, VACUUM wil set up page-level visibility bit
(PD_ALL_VISIBLE) if not previously set.
To end up with empty page which is missing visibility hint bit, we need
to forcefuly cancel (kill -9) backend, executing page freezing, jsut
after it did page pruning. Use injeciton point for this purpose and
add TAP test to cover "recovery" after error code path.
---
 src/backend/access/heap/vacuumlazy.c          |  6 ++
 .../test_misc/t/010_vacuum_empty_page.pl      | 75 +++++++++++++++++++
 2 files changed, 81 insertions(+)
 create mode 100644 src/test/modules/test_misc/t/010_vacuum_empty_page.pl

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 30778a15639..9b8cbb67f11 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -153,6 +153,7 @@
 #include "storage/freespace.h"
 #include "storage/lmgr.h"
 #include "storage/read_stream.h"
+#include "utils/injection_point.h"
 #include "utils/lsyscache.h"
 #include "utils/pg_rusage.h"
 #include "utils/timestamp.h"
@@ -1899,6 +1900,8 @@ lazy_scan_new_or_empty(LVRelState *vacrel, Buffer buf, BlockNumber blkno,
 		 */
 		if (!PageIsAllVisible(page))
 		{
+			INJECTION_POINT("vacuum-empty-page-non-all-vis", NULL);
+
 			START_CRIT_SECTION();
 
 			/* mark buffer dirty before writing a WAL record */
@@ -2012,6 +2015,9 @@ lazy_scan_prune(LVRelState *vacrel,
 							   &vacrel->offnum,
 							   &vacrel->NewRelfrozenXid, &vacrel->NewRelminMxid);
 
+	
+	INJECTION_POINT("vacuum-heap-prune-and-freeze-after", NULL);
+
 	Assert(MultiXactIdIsValid(vacrel->NewRelminMxid));
 	Assert(TransactionIdIsValid(vacrel->NewRelfrozenXid));
 
diff --git a/src/test/modules/test_misc/t/010_vacuum_empty_page.pl b/src/test/modules/test_misc/t/010_vacuum_empty_page.pl
new file mode 100644
index 00000000000..af5c39d2435
--- /dev/null
+++ b/src/test/modules/test_misc/t/010_vacuum_empty_page.pl
@@ -0,0 +1,75 @@
+# Copyright (c) 2025, PostgreSQL Global Development Group
+
+# Check how temporary file removals and statement queries are associated
+# in the server logs for various query sequences with the simple and
+# extended query protocols.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+if ($ENV{enable_injection_points} ne 'yes')
+{
+	plan skip_all => 'Injection points not supported by this build';
+}
+
+# Initialize a new PostgreSQL test cluster
+my $node = PostgreSQL::Test::Cluster->new('primary');
+$node->init();
+$node->append_conf(
+	'postgresql.conf', qq(
+log_min_messages = 'notice'
+));
+$node->start;
+
+# Check if the extension injection_points is available, as it may be
+# possible that this script is run with installcheck, where the module
+# would not be installed by default.
+if (!$node->check_extension('injection_points'))
+{
+	plan skip_all => 'Extension injection_points not installed';
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+
+# Setup table and populate with data
+$node->safe_psql(
+	"postgres", qq{
+CREATE TABLE vac_empty_test(a int);
+BEGIN;
+INSERT INTO vac_empty_test DEFAULT VALUES;
+ROLLBACK;
+});
+
+# From this point, autovacuum worker will wait at startup.
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('vacuum-heap-prune-and-freeze-after', 'error');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_attach('vacuum-empty-page-non-all-vis', 'notice');");
+
+$node->psql('postgres', "VACUUM (FREEZE) vac_empty_test;", on_error_stop => 1);
+
+my $offset = -s $node->logfile;
+
+# Run vacuum, force it on empty page. 
+$node->safe_psql(
+	"postgres", qq{
+VACUUM (DISABLE_PAGE_SKIPPING) vac_empty_test;
+});
+
+ok( $node->log_contains(
+		qr/NOTICE:  notice triggered for injection point vacuum-empty-page-non-all-vis/,
+		$offset),
+	"vacuum sets all-visible page bit for empty page");
+
+
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('vacuum-heap-prune-and-freeze-after');");
+$node->safe_psql('postgres',
+	"SELECT injection_points_detach('vacuum-empty-page-non-all-vis');");
+
+$node->stop('fast');
+done_testing();
-- 
2.43.0



view thread (143+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: eliminate xl_heap_visible to reduce WAL (and eventually set VM on-access)
  In-Reply-To: <CALdSSPgKH7TdW0zcDPjBxx+MX18bjc65K12KgoGtX6ZP8Qq63w@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox