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;