postgres.git / summary / log / commit / refs
commit 13243ae44cf6d81f666223d3385bdf0a824cd07d
Author: Melanie Plageman <melanieplageman@gmail.com>
Date: Wed Sep 23 20:33:25 2026 +0000
Correct on-access VM setting heuristic
The heuristic to avoid setting the VM during on-access pruning when
doing so would emit an extra FPI missed a few cases. First it missed
temp and unlogged tables. Those will never emit an FPI, so they can
always set the VM if the page is all-visible. It also missed that if
hint bits are not WAL-logged, setting only the VM passes REGBUF_NO_IMAGE
for an already WAL-logged heap page, forbidding a heap FPI in the WAL
record. A page that has never been WAL-logged still requires a heap FPI.
The third is more subtle: If the page is all-visible, the new prune xid
will be InvalidTransactionId. On-access pruning only executes when the
current pd_prune_xid is valid and visible. So, when on-access pruning
finds the page all-visible, it will always clear pd_prune_xid, modifying
the page.
Knowing this means we can set the VM without emitting an extra heap FPI
in more cases. When hint bits are WAL-logged, if the heap buffer is
clean, modifying pd_prune_xid will emit an FPI if one is required. There
is no reason to try to avoid an FPI by not setting the VM. So, set the
VM in this case. However, when the heap buffer is already dirty,
modifying pd_prune_xid can avoid an FPI; so if the page hasn't been
logged since the last checkpoint, setting it all-visible will emit an
extra heap page FPI. We will still avoid setting the VM in this case.
This is being backpatched to 19 because since 378a216187ae pd_prune_xid
is set on insert, and if we execute a prune cycle and skip setting the
VM because of an incorrect heuristic, we have added new wasted work in
PG 19.
This commit adds some tests covering these cases. It also updates one of
the existing temp table tests to avoid exceeding the pin limit. Setting
the VM and FSM on-access when querying temp tables takes more local pins
and can run into the limit with fewer heap buffers pinned.
Reported-by: Melanie Plageman <melanieplageman@gmail.com>
Author: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAAKRu_amj7qLF4c=9ijd=708Fu2G8gg-2EqwBu=aCdAHU2sPHg@mail.gmail.com
Backpatch-through: 19
contrib/pg_visibility/expected/pg_visibility.out | 78 ++++++++++++++++++++++++
contrib/pg_visibility/sql/pg_visibility.sql | 36 +++++++++++
src/backend/access/heap/pruneheap.c | 37 ++++++++---
src/test/regress/expected/temp.out | 2 +-
src/test/regress/sql/temp.sql | 2 +-
5 files changed, 146 insertions(+), 9 deletions(-)
diff --git a/contrib/pg_visibility/expected/pg_visibility.out b/contrib/pg_visibility/expected/pg_visibility.out
index d26f0ab7589..5f2e1ed7306 100644
--- a/contrib/pg_visibility/expected/pg_visibility.out
+++ b/contrib/pg_visibility/expected/pg_visibility.out
@@ -248,6 +248,84 @@ select pg_visibility_map_summary('test_vac_unmodified_heap');
(1,1)
(1 row)
+-- Test that on-access pruning during a read-only scan sets the VM. Temp
+-- tables are used because their visibility horizon depends only on this
+-- backend and no other process can pin their buffers, so the conditional
+-- cleanup lock needed for pruning is always available.
+create temp table test_on_access_vm(a int, b text) with (fillfactor = 90);
+insert into test_on_access_vm select g, repeat('x', 99)
+ from generate_series(1, 500) g;
+-- HOT-update a few rows on every page. The new versions fit in the space
+-- reserved by the fillfactor, and afterwards each page has too little free
+-- space to escape on-access pruning.
+update test_on_access_vm set b = b where a % 20 = 0;
+select pg_visibility_map_summary('test_on_access_vm');
+ pg_visibility_map_summary
+---------------------------
+ (0,0)
+(1 row)
+
+-- A read-only scan that prunes tuples sets the VM
+select count(*) from test_on_access_vm;
+ count
+-------
+ 500
+(1 row)
+
+select pg_visibility_map_summary('test_on_access_vm');
+ pg_visibility_map_summary
+---------------------------
+ (9,0)
+(1 row)
+
+select * from pg_check_visible('test_on_access_vm');
+ t_ctid
+--------
+(0 rows)
+
+-- Test that a read-only scan of newly inserted data sets the VM
+create temp table test_on_access_vm_insert_only(a int, b text);
+insert into test_on_access_vm_insert_only select g, repeat('x', 99)
+ from generate_series(1, 500) g;
+select pg_visibility_map_summary('test_on_access_vm_insert_only');
+ pg_visibility_map_summary
+---------------------------
+ (0,0)
+(1 row)
+
+select count(*) from test_on_access_vm_insert_only;
+ count
+-------
+ 500
+(1 row)
+
+select pg_visibility_map_summary('test_on_access_vm_insert_only');
+ pg_visibility_map_summary
+---------------------------
+ (8,0)
+(1 row)
+
+select * from pg_check_visible('test_on_access_vm_insert_only');
+ t_ctid
+--------
+(0 rows)
+
+-- Test that a scan of an UPDATE's target relation does not set the VM, even
+-- when no rows match.
+create temp table test_on_access_vm_modify(a int, b text) with (fillfactor = 90);
+insert into test_on_access_vm_modify select g, repeat('x', 99)
+ from generate_series(1, 500) g;
+-- Create some dead rows for the next update's on-access pruning to clean up.
+-- We need to actually do pruning to exercise the right code path.
+update test_on_access_vm_modify set b = b where a % 20 = 0;
+-- This matches no rows, but scans every page as the query's result relation.
+update test_on_access_vm_modify set b = b where a = -1;
+select pg_visibility_map_summary('test_on_access_vm_modify');
+ pg_visibility_map_summary
+---------------------------
+ (0,0)
+(1 row)
+
-- test copy freeze
create table copyfreeze (a int, b char(1500));
-- load all rows via COPY FREEZE and ensure that all pages are set all-visible
diff --git a/contrib/pg_visibility/sql/pg_visibility.sql b/contrib/pg_visibility/sql/pg_visibility.sql
index 0888adb96a6..7a90132bbff 100644
--- a/contrib/pg_visibility/sql/pg_visibility.sql
+++ b/contrib/pg_visibility/sql/pg_visibility.sql
@@ -114,6 +114,42 @@ SELECT (flags & x'0004'::int) <> 0
vacuum test_vac_unmodified_heap;
select pg_visibility_map_summary('test_vac_unmodified_heap');
+-- Test that on-access pruning during a read-only scan sets the VM. Temp
+-- tables are used because their visibility horizon depends only on this
+-- backend and no other process can pin their buffers, so the conditional
+-- cleanup lock needed for pruning is always available.
+create temp table test_on_access_vm(a int, b text) with (fillfactor = 90);
+insert into test_on_access_vm select g, repeat('x', 99)
+ from generate_series(1, 500) g;
+-- HOT-update a few rows on every page. The new versions fit in the space
+-- reserved by the fillfactor, and afterwards each page has too little free
+-- space to escape on-access pruning.
+update test_on_access_vm set b = b where a % 20 = 0;
+select pg_visibility_map_summary('test_on_access_vm');
+-- A read-only scan that prunes tuples sets the VM
+select count(*) from test_on_access_vm;
+select pg_visibility_map_summary('test_on_access_vm');
+select * from pg_check_visible('test_on_access_vm');
+-- Test that a read-only scan of newly inserted data sets the VM
+create temp table test_on_access_vm_insert_only(a int, b text);
+insert into test_on_access_vm_insert_only select g, repeat('x', 99)
+ from generate_series(1, 500) g;
+select pg_visibility_map_summary('test_on_access_vm_insert_only');
+select count(*) from test_on_access_vm_insert_only;
+select pg_visibility_map_summary('test_on_access_vm_insert_only');
+select * from pg_check_visible('test_on_access_vm_insert_only');
+-- Test that a scan of an UPDATE's target relation does not set the VM, even
+-- when no rows match.
+create temp table test_on_access_vm_modify(a int, b text) with (fillfactor = 90);
+insert into test_on_access_vm_modify select g, repeat('x', 99)
+ from generate_series(1, 500) g;
+-- Create some dead rows for the next update's on-access pruning to clean up.
+-- We need to actually do pruning to exercise the right code path.
+update test_on_access_vm_modify set b = b where a % 20 = 0;
+-- This matches no rows, but scans every page as the query's result relation.
+update test_on_access_vm_modify set b = b where a = -1;
+select pg_visibility_map_summary('test_on_access_vm_modify');
+
-- test copy freeze
create table copyfreeze (a int, b char(1500));
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 89ab2fb2fb9..833f6f0395c 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -991,16 +991,39 @@ heap_page_will_set_vm(PruneState *prstate, PruneReason reason,
return false;
/*
- * If this is an on-access call and we're not actually pruning, avoid
- * setting the visibility map if it would newly dirty the heap page or, if
- * the page is already dirty, if doing so would require including a
- * full-page image (FPI) of the heap page in the WAL.
+ * If this is an on-access call and we're not actually pruning or
+ * freezing, consider whether setting the VM would cost us an additional
+ * heap page FPI. If the relation isn't WAL-logged, or if hint bits are
+ * not WAL-logged, setting the VM won't include a heap page FPI (the
+ * latter passes REGBUF_NO_IMAGE for the heap page), apart from a page
+ * that has never been WAL-logged, which we don't bother about here.
*/
if (reason == PRUNE_ON_ACCESS && !do_prune && !do_freeze &&
- (!BufferIsDirty(prstate->buffer) || XLogCheckBufferNeedsBackup(prstate->buffer)))
+ RelationNeedsWAL(prstate->relation) && XLogHintBitIsNeeded())
{
- prstate->set_all_visible = prstate->set_all_frozen = false;
- return false;
+ /*
+ * Because the page is known to be all-visible, we will clear
+ * pd_prune_xid regardless of whether we actually set the page
+ * all-visible in the VM. That clear is a hint update which is not
+ * WAL-logged, other than an FPI for torn-page protection, so in some
+ * cases we want to avoid setting the VM if doing so would cost us a
+ * heap page FPI that clearing pd_prune_xid wouldn't have.
+ *
+ * Since hint bits are WAL-logged, if the buffer is clean, clearing
+ * pd_prune_xid will already emit a heap page FPI if one is needed, so
+ * there's no reason to avoid setting the VM.
+ *
+ * However, if the heap buffer is already dirty, clearing pd_prune_xid
+ * will never emit an FPI. So avoid setting the VM if the page hasn't
+ * been WAL-logged since the current checkpoint began, as the record
+ * setting the VM would then include a heap page FPI.
+ */
+ if (BufferIsDirty(prstate->buffer) &&
+ XLogCheckBufferNeedsBackup(prstate->buffer))
+ {
+ prstate->set_all_visible = prstate->set_all_frozen = false;
+ return false;
+ }
}
prstate->new_vmbits = VISIBILITYMAP_ALL_VISIBLE;
diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out
index a50c7ae88a9..bd4653aed27 100644
--- a/src/test/regress/expected/temp.out
+++ b/src/test/regress/expected/temp.out
@@ -485,7 +485,7 @@ FETCH NEXT FROM c_3;
(1 row)
-- new cursors with pins can be created after subtrans rollback
-SELECT test_temp_pin(10, 94);
+SELECT test_temp_pin(10, 90);
test_temp_pin
---------------
diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql
index d50472ddced..84978621830 100644
--- a/src/test/regress/sql/temp.sql
+++ b/src/test/regress/sql/temp.sql
@@ -369,7 +369,7 @@ ROLLBACK TO SAVEPOINT rescue_me;
FETCH NEXT FROM c_3;
-- new cursors with pins can be created after subtrans rollback
-SELECT test_temp_pin(10, 94);
+SELECT test_temp_pin(10, 90);
-- Check that read streams deal with lower number of pins available
SELECT count(*), max(a) max_a, min(a) min_a, max(cnt) max_cnt FROM test_temp;
[parent: f94f8b18d3b7]