public inbox for [email protected]  

help / color / mirror / Atom feed

Re: Wrong results from Parallel Hash Full Join
7+ messages / 1 participants
[nested] [flat]

* Re: Wrong results from Parallel Hash Full Join
@ 2023-04-20 15:49  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Melanie Plageman @ 2023-04-20 15:49 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Thomas Munro <[email protected]>; Richard Guo <[email protected]>; pgsql-hackers

On Wed, Apr 19, 2023 at 8:43 PM Melanie Plageman
<[email protected]> wrote:
> On Wed, Apr 19, 2023 at 3:20 PM Andres Freund <[email protected]> wrote:
>> On 2023-04-19 12:16:24 -0500, Justin Pryzby wrote:
>> > On Wed, Apr 19, 2023 at 11:17:04AM -0400, Melanie Plageman wrote:
>> > > Ultimately this is probably fine. If we wanted to modify one of the
>> > > existing tests to cover the multi-batch case, changing the select
>> > > count(*) to a select * would do the trick. I imagine we wouldn't want to
>> > > do this because of the excessive output this would produce. I wondered
>> > > if there was a pattern in the tests for getting around this.
>> >
>> > You could use explain (ANALYZE).  But the output is machine-dependant in
>> > various ways (which is why the tests use "explain analyze so rarely).
>>
>> I think with sufficient options it's not machine specific. We have a bunch of
>>  EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF) ..
>> in our tests.
>
>
> Cool. Yea, so ultimately these options are almost enough but memory
> usage changes from execution to execution. There are some tests which do
> regexp_replace() on the memory usage part of the EXPLAIN ANALYZE output
> to allow us to still compare the plans. However, I figured if I was
> already going to go to the trouble of using regexp_replace(), I might as
> well write a function that returns the "Actual Rows" field from the
> EXPLAIN ANALYZE output.
>
> The attached patch does that. I admittedly mostly copy-pasted the
> plpgsql function from similar examples in other tests, and I suspect it
> may be overkill and also poorly written.

I renamed the function to join_hash_actual_rows to avoid potentially
affecting other tests. Nothing about the function is specific to a hash
join plan, so I think it is more clear to prefix the function with the
test file name. v2 attached.

- Melanie


Attachments:

  [text/x-patch] v2-0001-Test-multi-batch-PHJ-match-bit-initialization.patch (12.6K, ../../CAAKRu_YPJkVsNwq-ejDV6P-NUdLOArOn-z22o7apqVC+8y04hA@mail.gmail.com/2-v2-0001-Test-multi-batch-PHJ-match-bit-initialization.patch)
  download | inline diff:
From d8f6946cf127092913d8f1b7a9741b97f3215bbd Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Wed, 19 Apr 2023 20:09:37 -0400
Subject: [PATCH v2] Test multi-batch PHJ match bit initialization

558c9d75 fixed a bug with initialization of the hash join match status
bit and added test coverage for serial hash join and single batch
parallel hash join. Add a test for multi-batch parallel hash join.

To test this with the existing relations in the join_hash test file,
this commit modifies some of the test queries to use SELECT * instead of
SELECT COUNT(*), which was needed to ensure that the tuples being
inserted into the hashtable had not already been made into virtual
tuples and lost their HOT status bit.

These modifications made the original tests introduced in 558c9d75
redundant.

Discussion: https://postgr.es/m/ZEAh6CewmARbDVuN%40telsasoft.com
---
 src/test/regress/expected/join_hash.out | 134 +++++++++++-------------
 src/test/regress/sql/join_hash.sql      |  72 +++++++------
 2 files changed, 100 insertions(+), 106 deletions(-)

diff --git a/src/test/regress/expected/join_hash.out b/src/test/regress/expected/join_hash.out
index e892e7cccb..d4a70f6754 100644
--- a/src/test/regress/expected/join_hash.out
+++ b/src/test/regress/expected/join_hash.out
@@ -49,10 +49,35 @@ begin
   end loop;
 end;
 $$;
+-- Return the number of actual rows returned by a query. This is useful when a
+-- count(*) would not execute the desired codepath but a SELECT * would produce
+-- an excessive number of rows.
+create or replace function join_hash_actual_rows(query text)
+returns jsonb language plpgsql
+as
+$$
+declare
+  elements jsonb;
+begin
+  execute 'explain (analyze, costs off, summary off, timing off, format ''json'') ' || query into strict elements;
+  if jsonb_array_length(elements) > 1 then
+    raise exception 'Cannot return actual rows from more than one plan';
+  end if;
+  return elements->0->'Plan'->'Actual Rows';
+end;
+$$;
 -- Make a simple relation with well distributed keys and correctly
 -- estimated size.
-create table simple as
-  select generate_series(1, 20000) AS id, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa';
+create table simple (id int, value text);
+insert into simple values (1, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa');
+-- Hash join reuses the HOT status bit to indicate match status. This can only
+-- be guaranteed to produce correct results if all the hash join tuple match
+-- bits are reset before reuse. This is done upon loading them into the
+-- hashtable. To test this, update simple, creating a HOT tuple. If this status
+-- bit isn't cleared, we won't correctly emit the NULL-extended unmatching
+-- tuple in full hash join.
+update simple set id = 1;
+insert into simple select generate_series(2, 20000), 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa';
 alter table simple set (parallel_workers = 2);
 analyze simple;
 -- Make a relation whose size we will under-estimate.  We want stats
@@ -273,6 +298,7 @@ set local max_parallel_workers_per_gather = 2;
 set local work_mem = '192kB';
 set local hash_mem_multiplier = 1.0;
 set local enable_parallel_hash = on;
+set local parallel_tuple_cost = 0;
 explain (costs off)
   select count(*) from simple r join simple s using (id);
                          QUERY PLAN                          
@@ -305,10 +331,11 @@ $$);
 (1 row)
 
 -- parallel full multi-batch hash join
-select count(*) from simple r full outer join simple s using (id);
- count 
--------
- 20000
+select join_hash_actual_rows(
+  'select * from simple r full outer join simple s on (r.id = 0 - s.id)');
+ join_hash_actual_rows 
+-----------------------
+ 40000
 (1 row)
 
 rollback to settings;
@@ -844,20 +871,20 @@ rollback to settings;
 savepoint settings;
 set local max_parallel_workers_per_gather = 0;
 explain (costs off)
-     select  count(*) from simple r full outer join simple s on (r.id = 0 - s.id);
-               QUERY PLAN               
-----------------------------------------
- Aggregate
-   ->  Hash Full Join
-         Hash Cond: ((0 - s.id) = r.id)
-         ->  Seq Scan on simple s
-         ->  Hash
-               ->  Seq Scan on simple r
-(6 rows)
+  select * from simple r full outer join simple s on (r.id = 0 - s.id);
+            QUERY PLAN            
+----------------------------------
+ Hash Full Join
+   Hash Cond: ((0 - s.id) = r.id)
+   ->  Seq Scan on simple s
+   ->  Hash
+         ->  Seq Scan on simple r
+(5 rows)
 
-select  count(*) from simple r full outer join simple s on (r.id = 0 - s.id);
- count 
--------
+select join_hash_actual_rows(
+  'select * from simple r full outer join simple s on (r.id = 0 - s.id)');
+ join_hash_actual_rows 
+-----------------------
  40000
 (1 row)
 
@@ -888,24 +915,24 @@ rollback to settings;
 -- parallelism is possible with parallel-aware full hash join
 savepoint settings;
 set local max_parallel_workers_per_gather = 2;
+set local parallel_tuple_cost = 0;
 explain (costs off)
-     select  count(*) from simple r full outer join simple s on (r.id = 0 - s.id);
-                         QUERY PLAN                          
--------------------------------------------------------------
- Finalize Aggregate
-   ->  Gather
-         Workers Planned: 2
-         ->  Partial Aggregate
-               ->  Parallel Hash Full Join
-                     Hash Cond: ((0 - s.id) = r.id)
-                     ->  Parallel Seq Scan on simple s
-                     ->  Parallel Hash
-                           ->  Parallel Seq Scan on simple r
-(9 rows)
+  select * from simple r full outer join simple s on (r.id = 0 - s.id);
+                   QUERY PLAN                    
+-------------------------------------------------
+ Gather
+   Workers Planned: 2
+   ->  Parallel Hash Full Join
+         Hash Cond: ((0 - s.id) = r.id)
+         ->  Parallel Seq Scan on simple s
+         ->  Parallel Hash
+               ->  Parallel Seq Scan on simple r
+(7 rows)
 
-select  count(*) from simple r full outer join simple s on (r.id = 0 - s.id);
- count 
--------
+select join_hash_actual_rows(
+  'select * from simple r full outer join simple s on (r.id = 0 - s.id)');
+ join_hash_actual_rows 
+-----------------------
  40000
 (1 row)
 
@@ -955,43 +982,6 @@ $$);
 (1 row)
 
 rollback to settings;
--- Hash join reuses the HOT status bit to indicate match status. This can only
--- be guaranteed to produce correct results if all the hash join tuple match
--- bits are reset before reuse. This is done upon loading them into the
--- hashtable.
-SAVEPOINT settings;
-SET enable_parallel_hash = on;
-SET min_parallel_table_scan_size = 0;
-SET parallel_setup_cost = 0;
-SET parallel_tuple_cost = 0;
-CREATE TABLE hjtest_matchbits_t1(id int);
-CREATE TABLE hjtest_matchbits_t2(id int);
-INSERT INTO hjtest_matchbits_t1 VALUES (1);
-INSERT INTO hjtest_matchbits_t2 VALUES (2);
--- Update should create a HOT tuple. If this status bit isn't cleared, we won't
--- correctly emit the NULL-extended unmatching tuple in full hash join.
-UPDATE hjtest_matchbits_t2 set id = 2;
-SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON t1.id = t2.id;
- id | id 
-----+----
-  1 |   
-    |  2
-(2 rows)
-
--- Test serial full hash join.
--- Resetting parallel_setup_cost should force a serial plan.
--- Just to be safe, however, set enable_parallel_hash to off, as parallel full
--- hash joins are only supported with shared hashtables.
-RESET parallel_setup_cost;
-SET enable_parallel_hash = off;
-SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON t1.id = t2.id;
- id | id 
-----+----
-  1 |   
-    |  2
-(2 rows)
-
-ROLLBACK TO settings;
 rollback;
 -- Verify that hash key expressions reference the correct
 -- nodes. Hashjoin's hashkeys need to reference its outer plan, Hash's
diff --git a/src/test/regress/sql/join_hash.sql b/src/test/regress/sql/join_hash.sql
index 06bab7a4c7..ef30afd747 100644
--- a/src/test/regress/sql/join_hash.sql
+++ b/src/test/regress/sql/join_hash.sql
@@ -53,10 +53,36 @@ begin
 end;
 $$;
 
+-- Return the number of actual rows returned by a query. This is useful when a
+-- count(*) would not execute the desired codepath but a SELECT * would produce
+-- an excessive number of rows.
+create or replace function join_hash_actual_rows(query text)
+returns jsonb language plpgsql
+as
+$$
+declare
+  elements jsonb;
+begin
+  execute 'explain (analyze, costs off, summary off, timing off, format ''json'') ' || query into strict elements;
+  if jsonb_array_length(elements) > 1 then
+    raise exception 'Cannot return actual rows from more than one plan';
+  end if;
+  return elements->0->'Plan'->'Actual Rows';
+end;
+$$;
+
 -- Make a simple relation with well distributed keys and correctly
 -- estimated size.
-create table simple as
-  select generate_series(1, 20000) AS id, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa';
+create table simple (id int, value text);
+insert into simple values (1, 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa');
+-- Hash join reuses the HOT status bit to indicate match status. This can only
+-- be guaranteed to produce correct results if all the hash join tuple match
+-- bits are reset before reuse. This is done upon loading them into the
+-- hashtable. To test this, update simple, creating a HOT tuple. If this status
+-- bit isn't cleared, we won't correctly emit the NULL-extended unmatching
+-- tuple in full hash join.
+update simple set id = 1;
+insert into simple select generate_series(2, 20000), 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa';
 alter table simple set (parallel_workers = 2);
 analyze simple;
 
@@ -179,6 +205,7 @@ set local max_parallel_workers_per_gather = 2;
 set local work_mem = '192kB';
 set local hash_mem_multiplier = 1.0;
 set local enable_parallel_hash = on;
+set local parallel_tuple_cost = 0;
 explain (costs off)
   select count(*) from simple r join simple s using (id);
 select count(*) from simple r join simple s using (id);
@@ -188,7 +215,8 @@ $$
   select count(*) from simple r join simple s using (id);
 $$);
 -- parallel full multi-batch hash join
-select count(*) from simple r full outer join simple s using (id);
+select join_hash_actual_rows(
+  'select * from simple r full outer join simple s on (r.id = 0 - s.id)');
 rollback to settings;
 
 -- The "bad" case: during execution we need to increase number of
@@ -460,8 +488,9 @@ rollback to settings;
 savepoint settings;
 set local max_parallel_workers_per_gather = 0;
 explain (costs off)
-     select  count(*) from simple r full outer join simple s on (r.id = 0 - s.id);
-select  count(*) from simple r full outer join simple s on (r.id = 0 - s.id);
+  select * from simple r full outer join simple s on (r.id = 0 - s.id);
+select join_hash_actual_rows(
+  'select * from simple r full outer join simple s on (r.id = 0 - s.id)');
 rollback to settings;
 
 -- parallelism not possible with parallel-oblivious full hash join
@@ -476,9 +505,11 @@ rollback to settings;
 -- parallelism is possible with parallel-aware full hash join
 savepoint settings;
 set local max_parallel_workers_per_gather = 2;
+set local parallel_tuple_cost = 0;
 explain (costs off)
-     select  count(*) from simple r full outer join simple s on (r.id = 0 - s.id);
-select  count(*) from simple r full outer join simple s on (r.id = 0 - s.id);
+  select * from simple r full outer join simple s on (r.id = 0 - s.id);
+select join_hash_actual_rows(
+  'select * from simple r full outer join simple s on (r.id = 0 - s.id)');
 rollback to settings;
 
 
@@ -506,33 +537,6 @@ $$
 $$);
 rollback to settings;
 
-
--- Hash join reuses the HOT status bit to indicate match status. This can only
--- be guaranteed to produce correct results if all the hash join tuple match
--- bits are reset before reuse. This is done upon loading them into the
--- hashtable.
-SAVEPOINT settings;
-SET enable_parallel_hash = on;
-SET min_parallel_table_scan_size = 0;
-SET parallel_setup_cost = 0;
-SET parallel_tuple_cost = 0;
-CREATE TABLE hjtest_matchbits_t1(id int);
-CREATE TABLE hjtest_matchbits_t2(id int);
-INSERT INTO hjtest_matchbits_t1 VALUES (1);
-INSERT INTO hjtest_matchbits_t2 VALUES (2);
--- Update should create a HOT tuple. If this status bit isn't cleared, we won't
--- correctly emit the NULL-extended unmatching tuple in full hash join.
-UPDATE hjtest_matchbits_t2 set id = 2;
-SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON t1.id = t2.id;
--- Test serial full hash join.
--- Resetting parallel_setup_cost should force a serial plan.
--- Just to be safe, however, set enable_parallel_hash to off, as parallel full
--- hash joins are only supported with shared hashtables.
-RESET parallel_setup_cost;
-SET enable_parallel_hash = off;
-SELECT * FROM hjtest_matchbits_t1 t1 FULL JOIN hjtest_matchbits_t2 t2 ON t1.id = t2.id;
-ROLLBACK TO settings;
-
 rollback;
 
 -- Verify that hash key expressions reference the correct
-- 
2.37.2



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v2 10/17] Inline heap_freeze_execute_prepared()
@ 2024-01-07 22:03  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Melanie Plageman @ 2024-01-07 22:03 UTC (permalink / raw)

In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune(). The original function,
heap_freeze_execute_prepared() is retained because the "no prune" case
in heap_page_prune() must still be able to emit a freeze record.
---
 src/backend/access/heap/heapam.c    | 61 +++++++++++++++++------------
 src/backend/access/heap/pruneheap.c | 51 ++++++++++++++++++++++--
 src/include/access/heapam.h         |  8 ++++
 3 files changed, 90 insertions(+), 30 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16e3f2520a4..a3691584c55 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -91,9 +91,6 @@ static void compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
 static TM_Result heap_lock_updated_tuple(Relation rel, HeapTuple tuple,
 										 ItemPointer ctid, TransactionId xid,
 										 LockTupleMode mode);
-static int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
-								 xl_heap_freeze_plan *plans_out,
-								 OffsetNumber *offsets_out);
 static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 								   uint16 *new_infomask2);
 static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
@@ -6713,30 +6710,17 @@ heap_pre_freeze_checks(Buffer buffer,
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
  */
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	START_CRIT_SECTION();
-
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6746,6 +6730,29 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		htup = (HeapTupleHeader) PageGetItem(page, itemid);
 		heap_execute_freeze_tuple(htup, frz);
 	}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Execute freezing of prepared tuples and WAL-logs the changes so that VACUUM
+ * can advance the rel's relfrozenxid later on without any risk of unsafe
+ * pg_xact lookups, even following a hard crash (or when querying from a
+ * standby).  We represent freezing by setting infomask bits in tuple headers,
+ * but this shouldn't be thought of as a hint. See section on buffer access
+ * rules in src/backend/storage/buffer/README. Must be called from within a
+ * critical section.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+							 TransactionId snapshotConflictHorizon,
+							 HeapTupleFreeze *tuples, int ntuples)
+{
+	Page		page = BufferGetPage(buffer);
+
+	Assert(ntuples > 0);
+
+	heap_freeze_prepared_tuples(buffer, tuples, ntuples);
 
 	MarkBufferDirty(buffer);
 
@@ -6758,7 +6765,11 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		xl_heap_freeze_page xlrec;
 		XLogRecPtr	recptr;
 
-		/* Prepare deduplicated representation for use in WAL record */
+		/*
+		 * Prepare deduplicated representation for use in WAL record
+		 * Destructively sorts tuples array in-place, so caller had better be
+		 * done with it.
+		 */
 		nplans = heap_log_freeze_plan(tuples, ntuples, plans, offsets);
 
 		xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
@@ -6783,8 +6794,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 
 		PageSetLSN(page, recptr);
 	}
-
-	END_CRIT_SECTION();
 }
 
 /*
@@ -6874,7 +6883,7 @@ heap_log_freeze_new_plan(xl_heap_freeze_plan *plan, HeapTupleFreeze *frz)
  * (actually there is one array per freeze plan, but that's not of immediate
  * concern to our caller).
  */
-static int
+int
 heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
 					 xl_heap_freeze_plan *plans_out,
 					 OffsetNumber *offsets_out)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bac461940de..d8b7eea5c21 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -613,10 +613,53 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(relation, buffer,
-									 frz_conflict_horizon,
-									 frozen, presult->nfrozen);
+		START_CRIT_SECTION();
+
+		Assert(presult->nfrozen > 0);
+
+		heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
+		MarkBufferDirty(buffer);
+
+		/* Now WAL-log freezing if necessary */
+		if (RelationNeedsWAL(relation))
+		{
+			xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+			OffsetNumber offsets[MaxHeapTuplesPerPage];
+			int			nplans;
+			xl_heap_freeze_page xlrec;
+			XLogRecPtr	recptr;
+
+			/*
+			 * Prepare deduplicated representation for use in WAL record
+			 * Destructively sorts tuples array in-place.
+			 */
+			nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
+
+			xlrec.snapshotConflictHorizon = frz_conflict_horizon;
+			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+			xlrec.nplans = nplans;
+
+			XLogBeginInsert();
+			XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+
+			/*
+			 * The freeze plan array and offset array are not actually in the
+			 * buffer, but pretend that they are.  When XLogInsert stores the
+			 * whole buffer, the arrays need not be stored too.
+			 */
+			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+			XLogRegisterBufData(0, (char *) plans,
+								nplans * sizeof(xl_heap_freeze_plan));
+			XLogRegisterBufData(0, (char *) offsets,
+								presult->nfrozen * sizeof(OffsetNumber));
+
+			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+
+			PageSetLSN(page, recptr);
+		}
+
+		END_CRIT_SECTION();
 	}
 	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
 	{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index d14f36d9ce7..41ebbb9f931 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
 #ifndef HEAPAM_H
 #define HEAPAM_H
 
+#include "access/heapam_xlog.h"
 #include "access/relation.h"	/* for backward compatibility */
 #include "access/relscan.h"
 #include "access/sdir.h"
@@ -321,9 +322,16 @@ extern void heap_pre_freeze_checks(Buffer buffer,
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 										 TransactionId snapshotConflictHorizon,
 										 HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+										HeapTupleFreeze *tuples, int ntuples);
 extern bool heap_freeze_tuple(HeapTupleHeader tuple,
 							  TransactionId relfrozenxid, TransactionId relminmxid,
 							  TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
+extern int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
+								 xl_heap_freeze_plan *plans_out,
+								 OffsetNumber *offsets_out);
 extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
 									 const struct VacuumCutoffs *cutoffs,
 									 TransactionId *NoFreezePageRelfrozenXid,
-- 
2.40.1


--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0011-Exit-heap_page_prune-early-if-no-prune.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v3 10/17] Inline heap_freeze_execute_prepared()
@ 2024-01-07 22:03  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Melanie Plageman @ 2024-01-07 22:03 UTC (permalink / raw)

In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune(). The original function,
heap_freeze_execute_prepared() is retained because the "no prune" case
in heap_page_prune() must still be able to emit a freeze record.
---
 src/backend/access/heap/heapam.c    | 61 +++++++++++++++++------------
 src/backend/access/heap/pruneheap.c | 51 ++++++++++++++++++++++--
 src/include/access/heapam.h         |  8 ++++
 3 files changed, 90 insertions(+), 30 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16e3f2520a4..a3691584c55 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -91,9 +91,6 @@ static void compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
 static TM_Result heap_lock_updated_tuple(Relation rel, HeapTuple tuple,
 										 ItemPointer ctid, TransactionId xid,
 										 LockTupleMode mode);
-static int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
-								 xl_heap_freeze_plan *plans_out,
-								 OffsetNumber *offsets_out);
 static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 								   uint16 *new_infomask2);
 static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
@@ -6713,30 +6710,17 @@ heap_pre_freeze_checks(Buffer buffer,
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
  */
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	START_CRIT_SECTION();
-
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6746,6 +6730,29 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		htup = (HeapTupleHeader) PageGetItem(page, itemid);
 		heap_execute_freeze_tuple(htup, frz);
 	}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Execute freezing of prepared tuples and WAL-logs the changes so that VACUUM
+ * can advance the rel's relfrozenxid later on without any risk of unsafe
+ * pg_xact lookups, even following a hard crash (or when querying from a
+ * standby).  We represent freezing by setting infomask bits in tuple headers,
+ * but this shouldn't be thought of as a hint. See section on buffer access
+ * rules in src/backend/storage/buffer/README. Must be called from within a
+ * critical section.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+							 TransactionId snapshotConflictHorizon,
+							 HeapTupleFreeze *tuples, int ntuples)
+{
+	Page		page = BufferGetPage(buffer);
+
+	Assert(ntuples > 0);
+
+	heap_freeze_prepared_tuples(buffer, tuples, ntuples);
 
 	MarkBufferDirty(buffer);
 
@@ -6758,7 +6765,11 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		xl_heap_freeze_page xlrec;
 		XLogRecPtr	recptr;
 
-		/* Prepare deduplicated representation for use in WAL record */
+		/*
+		 * Prepare deduplicated representation for use in WAL record
+		 * Destructively sorts tuples array in-place, so caller had better be
+		 * done with it.
+		 */
 		nplans = heap_log_freeze_plan(tuples, ntuples, plans, offsets);
 
 		xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
@@ -6783,8 +6794,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 
 		PageSetLSN(page, recptr);
 	}
-
-	END_CRIT_SECTION();
 }
 
 /*
@@ -6874,7 +6883,7 @@ heap_log_freeze_new_plan(xl_heap_freeze_plan *plan, HeapTupleFreeze *frz)
  * (actually there is one array per freeze plan, but that's not of immediate
  * concern to our caller).
  */
-static int
+int
 heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
 					 xl_heap_freeze_plan *plans_out,
 					 OffsetNumber *offsets_out)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bc0a23da61b..d4356e0bce9 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -627,10 +627,53 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(relation, buffer,
-									 frz_conflict_horizon,
-									 frozen, presult->nfrozen);
+		START_CRIT_SECTION();
+
+		Assert(presult->nfrozen > 0);
+
+		heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
+		MarkBufferDirty(buffer);
+
+		/* Now WAL-log freezing if necessary */
+		if (RelationNeedsWAL(relation))
+		{
+			xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+			OffsetNumber offsets[MaxHeapTuplesPerPage];
+			int			nplans;
+			xl_heap_freeze_page xlrec;
+			XLogRecPtr	recptr;
+
+			/*
+			 * Prepare deduplicated representation for use in WAL record
+			 * Destructively sorts tuples array in-place.
+			 */
+			nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
+
+			xlrec.snapshotConflictHorizon = frz_conflict_horizon;
+			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+			xlrec.nplans = nplans;
+
+			XLogBeginInsert();
+			XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+
+			/*
+			 * The freeze plan array and offset array are not actually in the
+			 * buffer, but pretend that they are.  When XLogInsert stores the
+			 * whole buffer, the arrays need not be stored too.
+			 */
+			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+			XLogRegisterBufData(0, (char *) plans,
+								nplans * sizeof(xl_heap_freeze_plan));
+			XLogRegisterBufData(0, (char *) offsets,
+								presult->nfrozen * sizeof(OffsetNumber));
+
+			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+
+			PageSetLSN(page, recptr);
+		}
+
+		END_CRIT_SECTION();
 	}
 	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
 	{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index dffbbd3cd3e..8a6bc071345 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
 #ifndef HEAPAM_H
 #define HEAPAM_H
 
+#include "access/heapam_xlog.h"
 #include "access/relation.h"	/* for backward compatibility */
 #include "access/relscan.h"
 #include "access/sdir.h"
@@ -320,9 +321,16 @@ extern void heap_pre_freeze_checks(Buffer buffer,
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 										 TransactionId snapshotConflictHorizon,
 										 HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+										HeapTupleFreeze *tuples, int ntuples);
 extern bool heap_freeze_tuple(HeapTupleHeader tuple,
 							  TransactionId relfrozenxid, TransactionId relminmxid,
 							  TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
+extern int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
+								 xl_heap_freeze_plan *plans_out,
+								 OffsetNumber *offsets_out);
 extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
 									 const struct VacuumCutoffs *cutoffs,
 									 TransactionId *NoFreezePageRelfrozenXid,
-- 
2.40.1


--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0011-Exit-heap_page_prune-early-if-no-prune.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v2 10/17] Inline heap_freeze_execute_prepared()
@ 2024-01-07 22:03  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Melanie Plageman @ 2024-01-07 22:03 UTC (permalink / raw)

In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune(). The original function,
heap_freeze_execute_prepared() is retained because the "no prune" case
in heap_page_prune() must still be able to emit a freeze record.
---
 src/backend/access/heap/heapam.c    | 61 +++++++++++++++++------------
 src/backend/access/heap/pruneheap.c | 51 ++++++++++++++++++++++--
 src/include/access/heapam.h         |  8 ++++
 3 files changed, 90 insertions(+), 30 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16e3f2520a4..a3691584c55 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -91,9 +91,6 @@ static void compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
 static TM_Result heap_lock_updated_tuple(Relation rel, HeapTuple tuple,
 										 ItemPointer ctid, TransactionId xid,
 										 LockTupleMode mode);
-static int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
-								 xl_heap_freeze_plan *plans_out,
-								 OffsetNumber *offsets_out);
 static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 								   uint16 *new_infomask2);
 static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
@@ -6713,30 +6710,17 @@ heap_pre_freeze_checks(Buffer buffer,
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
  */
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	START_CRIT_SECTION();
-
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6746,6 +6730,29 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		htup = (HeapTupleHeader) PageGetItem(page, itemid);
 		heap_execute_freeze_tuple(htup, frz);
 	}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Execute freezing of prepared tuples and WAL-logs the changes so that VACUUM
+ * can advance the rel's relfrozenxid later on without any risk of unsafe
+ * pg_xact lookups, even following a hard crash (or when querying from a
+ * standby).  We represent freezing by setting infomask bits in tuple headers,
+ * but this shouldn't be thought of as a hint. See section on buffer access
+ * rules in src/backend/storage/buffer/README. Must be called from within a
+ * critical section.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+							 TransactionId snapshotConflictHorizon,
+							 HeapTupleFreeze *tuples, int ntuples)
+{
+	Page		page = BufferGetPage(buffer);
+
+	Assert(ntuples > 0);
+
+	heap_freeze_prepared_tuples(buffer, tuples, ntuples);
 
 	MarkBufferDirty(buffer);
 
@@ -6758,7 +6765,11 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		xl_heap_freeze_page xlrec;
 		XLogRecPtr	recptr;
 
-		/* Prepare deduplicated representation for use in WAL record */
+		/*
+		 * Prepare deduplicated representation for use in WAL record
+		 * Destructively sorts tuples array in-place, so caller had better be
+		 * done with it.
+		 */
 		nplans = heap_log_freeze_plan(tuples, ntuples, plans, offsets);
 
 		xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
@@ -6783,8 +6794,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 
 		PageSetLSN(page, recptr);
 	}
-
-	END_CRIT_SECTION();
 }
 
 /*
@@ -6874,7 +6883,7 @@ heap_log_freeze_new_plan(xl_heap_freeze_plan *plan, HeapTupleFreeze *frz)
  * (actually there is one array per freeze plan, but that's not of immediate
  * concern to our caller).
  */
-static int
+int
 heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
 					 xl_heap_freeze_plan *plans_out,
 					 OffsetNumber *offsets_out)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bac461940de..d8b7eea5c21 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -613,10 +613,53 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(relation, buffer,
-									 frz_conflict_horizon,
-									 frozen, presult->nfrozen);
+		START_CRIT_SECTION();
+
+		Assert(presult->nfrozen > 0);
+
+		heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
+		MarkBufferDirty(buffer);
+
+		/* Now WAL-log freezing if necessary */
+		if (RelationNeedsWAL(relation))
+		{
+			xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+			OffsetNumber offsets[MaxHeapTuplesPerPage];
+			int			nplans;
+			xl_heap_freeze_page xlrec;
+			XLogRecPtr	recptr;
+
+			/*
+			 * Prepare deduplicated representation for use in WAL record
+			 * Destructively sorts tuples array in-place.
+			 */
+			nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
+
+			xlrec.snapshotConflictHorizon = frz_conflict_horizon;
+			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+			xlrec.nplans = nplans;
+
+			XLogBeginInsert();
+			XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+
+			/*
+			 * The freeze plan array and offset array are not actually in the
+			 * buffer, but pretend that they are.  When XLogInsert stores the
+			 * whole buffer, the arrays need not be stored too.
+			 */
+			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+			XLogRegisterBufData(0, (char *) plans,
+								nplans * sizeof(xl_heap_freeze_plan));
+			XLogRegisterBufData(0, (char *) offsets,
+								presult->nfrozen * sizeof(OffsetNumber));
+
+			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+
+			PageSetLSN(page, recptr);
+		}
+
+		END_CRIT_SECTION();
 	}
 	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
 	{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index d14f36d9ce7..41ebbb9f931 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
 #ifndef HEAPAM_H
 #define HEAPAM_H
 
+#include "access/heapam_xlog.h"
 #include "access/relation.h"	/* for backward compatibility */
 #include "access/relscan.h"
 #include "access/sdir.h"
@@ -321,9 +322,16 @@ extern void heap_pre_freeze_checks(Buffer buffer,
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 										 TransactionId snapshotConflictHorizon,
 										 HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+										HeapTupleFreeze *tuples, int ntuples);
 extern bool heap_freeze_tuple(HeapTupleHeader tuple,
 							  TransactionId relfrozenxid, TransactionId relminmxid,
 							  TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
+extern int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
+								 xl_heap_freeze_plan *plans_out,
+								 OffsetNumber *offsets_out);
 extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
 									 const struct VacuumCutoffs *cutoffs,
 									 TransactionId *NoFreezePageRelfrozenXid,
-- 
2.40.1


--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0011-Exit-heap_page_prune-early-if-no-prune.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v3 10/17] Inline heap_freeze_execute_prepared()
@ 2024-01-07 22:03  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Melanie Plageman @ 2024-01-07 22:03 UTC (permalink / raw)

In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune(). The original function,
heap_freeze_execute_prepared() is retained because the "no prune" case
in heap_page_prune() must still be able to emit a freeze record.
---
 src/backend/access/heap/heapam.c    | 61 +++++++++++++++++------------
 src/backend/access/heap/pruneheap.c | 51 ++++++++++++++++++++++--
 src/include/access/heapam.h         |  8 ++++
 3 files changed, 90 insertions(+), 30 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16e3f2520a4..a3691584c55 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -91,9 +91,6 @@ static void compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
 static TM_Result heap_lock_updated_tuple(Relation rel, HeapTuple tuple,
 										 ItemPointer ctid, TransactionId xid,
 										 LockTupleMode mode);
-static int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
-								 xl_heap_freeze_plan *plans_out,
-								 OffsetNumber *offsets_out);
 static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 								   uint16 *new_infomask2);
 static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
@@ -6713,30 +6710,17 @@ heap_pre_freeze_checks(Buffer buffer,
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
  */
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	START_CRIT_SECTION();
-
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6746,6 +6730,29 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		htup = (HeapTupleHeader) PageGetItem(page, itemid);
 		heap_execute_freeze_tuple(htup, frz);
 	}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Execute freezing of prepared tuples and WAL-logs the changes so that VACUUM
+ * can advance the rel's relfrozenxid later on without any risk of unsafe
+ * pg_xact lookups, even following a hard crash (or when querying from a
+ * standby).  We represent freezing by setting infomask bits in tuple headers,
+ * but this shouldn't be thought of as a hint. See section on buffer access
+ * rules in src/backend/storage/buffer/README. Must be called from within a
+ * critical section.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+							 TransactionId snapshotConflictHorizon,
+							 HeapTupleFreeze *tuples, int ntuples)
+{
+	Page		page = BufferGetPage(buffer);
+
+	Assert(ntuples > 0);
+
+	heap_freeze_prepared_tuples(buffer, tuples, ntuples);
 
 	MarkBufferDirty(buffer);
 
@@ -6758,7 +6765,11 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		xl_heap_freeze_page xlrec;
 		XLogRecPtr	recptr;
 
-		/* Prepare deduplicated representation for use in WAL record */
+		/*
+		 * Prepare deduplicated representation for use in WAL record
+		 * Destructively sorts tuples array in-place, so caller had better be
+		 * done with it.
+		 */
 		nplans = heap_log_freeze_plan(tuples, ntuples, plans, offsets);
 
 		xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
@@ -6783,8 +6794,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 
 		PageSetLSN(page, recptr);
 	}
-
-	END_CRIT_SECTION();
 }
 
 /*
@@ -6874,7 +6883,7 @@ heap_log_freeze_new_plan(xl_heap_freeze_plan *plan, HeapTupleFreeze *frz)
  * (actually there is one array per freeze plan, but that's not of immediate
  * concern to our caller).
  */
-static int
+int
 heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
 					 xl_heap_freeze_plan *plans_out,
 					 OffsetNumber *offsets_out)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bc0a23da61b..d4356e0bce9 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -627,10 +627,53 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(relation, buffer,
-									 frz_conflict_horizon,
-									 frozen, presult->nfrozen);
+		START_CRIT_SECTION();
+
+		Assert(presult->nfrozen > 0);
+
+		heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
+		MarkBufferDirty(buffer);
+
+		/* Now WAL-log freezing if necessary */
+		if (RelationNeedsWAL(relation))
+		{
+			xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+			OffsetNumber offsets[MaxHeapTuplesPerPage];
+			int			nplans;
+			xl_heap_freeze_page xlrec;
+			XLogRecPtr	recptr;
+
+			/*
+			 * Prepare deduplicated representation for use in WAL record
+			 * Destructively sorts tuples array in-place.
+			 */
+			nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
+
+			xlrec.snapshotConflictHorizon = frz_conflict_horizon;
+			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+			xlrec.nplans = nplans;
+
+			XLogBeginInsert();
+			XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+
+			/*
+			 * The freeze plan array and offset array are not actually in the
+			 * buffer, but pretend that they are.  When XLogInsert stores the
+			 * whole buffer, the arrays need not be stored too.
+			 */
+			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+			XLogRegisterBufData(0, (char *) plans,
+								nplans * sizeof(xl_heap_freeze_plan));
+			XLogRegisterBufData(0, (char *) offsets,
+								presult->nfrozen * sizeof(OffsetNumber));
+
+			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+
+			PageSetLSN(page, recptr);
+		}
+
+		END_CRIT_SECTION();
 	}
 	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
 	{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index dffbbd3cd3e..8a6bc071345 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
 #ifndef HEAPAM_H
 #define HEAPAM_H
 
+#include "access/heapam_xlog.h"
 #include "access/relation.h"	/* for backward compatibility */
 #include "access/relscan.h"
 #include "access/sdir.h"
@@ -320,9 +321,16 @@ extern void heap_pre_freeze_checks(Buffer buffer,
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 										 TransactionId snapshotConflictHorizon,
 										 HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+										HeapTupleFreeze *tuples, int ntuples);
 extern bool heap_freeze_tuple(HeapTupleHeader tuple,
 							  TransactionId relfrozenxid, TransactionId relminmxid,
 							  TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
+extern int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
+								 xl_heap_freeze_plan *plans_out,
+								 OffsetNumber *offsets_out);
 extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
 									 const struct VacuumCutoffs *cutoffs,
 									 TransactionId *NoFreezePageRelfrozenXid,
-- 
2.40.1


--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0011-Exit-heap_page_prune-early-if-no-prune.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v2 10/17] Inline heap_freeze_execute_prepared()
@ 2024-01-07 22:03  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Melanie Plageman @ 2024-01-07 22:03 UTC (permalink / raw)

In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune(). The original function,
heap_freeze_execute_prepared() is retained because the "no prune" case
in heap_page_prune() must still be able to emit a freeze record.
---
 src/backend/access/heap/heapam.c    | 61 +++++++++++++++++------------
 src/backend/access/heap/pruneheap.c | 51 ++++++++++++++++++++++--
 src/include/access/heapam.h         |  8 ++++
 3 files changed, 90 insertions(+), 30 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16e3f2520a4..a3691584c55 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -91,9 +91,6 @@ static void compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
 static TM_Result heap_lock_updated_tuple(Relation rel, HeapTuple tuple,
 										 ItemPointer ctid, TransactionId xid,
 										 LockTupleMode mode);
-static int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
-								 xl_heap_freeze_plan *plans_out,
-								 OffsetNumber *offsets_out);
 static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 								   uint16 *new_infomask2);
 static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
@@ -6713,30 +6710,17 @@ heap_pre_freeze_checks(Buffer buffer,
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
  */
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	START_CRIT_SECTION();
-
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6746,6 +6730,29 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		htup = (HeapTupleHeader) PageGetItem(page, itemid);
 		heap_execute_freeze_tuple(htup, frz);
 	}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Execute freezing of prepared tuples and WAL-logs the changes so that VACUUM
+ * can advance the rel's relfrozenxid later on without any risk of unsafe
+ * pg_xact lookups, even following a hard crash (or when querying from a
+ * standby).  We represent freezing by setting infomask bits in tuple headers,
+ * but this shouldn't be thought of as a hint. See section on buffer access
+ * rules in src/backend/storage/buffer/README. Must be called from within a
+ * critical section.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+							 TransactionId snapshotConflictHorizon,
+							 HeapTupleFreeze *tuples, int ntuples)
+{
+	Page		page = BufferGetPage(buffer);
+
+	Assert(ntuples > 0);
+
+	heap_freeze_prepared_tuples(buffer, tuples, ntuples);
 
 	MarkBufferDirty(buffer);
 
@@ -6758,7 +6765,11 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		xl_heap_freeze_page xlrec;
 		XLogRecPtr	recptr;
 
-		/* Prepare deduplicated representation for use in WAL record */
+		/*
+		 * Prepare deduplicated representation for use in WAL record
+		 * Destructively sorts tuples array in-place, so caller had better be
+		 * done with it.
+		 */
 		nplans = heap_log_freeze_plan(tuples, ntuples, plans, offsets);
 
 		xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
@@ -6783,8 +6794,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 
 		PageSetLSN(page, recptr);
 	}
-
-	END_CRIT_SECTION();
 }
 
 /*
@@ -6874,7 +6883,7 @@ heap_log_freeze_new_plan(xl_heap_freeze_plan *plan, HeapTupleFreeze *frz)
  * (actually there is one array per freeze plan, but that's not of immediate
  * concern to our caller).
  */
-static int
+int
 heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
 					 xl_heap_freeze_plan *plans_out,
 					 OffsetNumber *offsets_out)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bac461940de..d8b7eea5c21 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -613,10 +613,53 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(relation, buffer,
-									 frz_conflict_horizon,
-									 frozen, presult->nfrozen);
+		START_CRIT_SECTION();
+
+		Assert(presult->nfrozen > 0);
+
+		heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
+		MarkBufferDirty(buffer);
+
+		/* Now WAL-log freezing if necessary */
+		if (RelationNeedsWAL(relation))
+		{
+			xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+			OffsetNumber offsets[MaxHeapTuplesPerPage];
+			int			nplans;
+			xl_heap_freeze_page xlrec;
+			XLogRecPtr	recptr;
+
+			/*
+			 * Prepare deduplicated representation for use in WAL record
+			 * Destructively sorts tuples array in-place.
+			 */
+			nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
+
+			xlrec.snapshotConflictHorizon = frz_conflict_horizon;
+			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+			xlrec.nplans = nplans;
+
+			XLogBeginInsert();
+			XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+
+			/*
+			 * The freeze plan array and offset array are not actually in the
+			 * buffer, but pretend that they are.  When XLogInsert stores the
+			 * whole buffer, the arrays need not be stored too.
+			 */
+			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+			XLogRegisterBufData(0, (char *) plans,
+								nplans * sizeof(xl_heap_freeze_plan));
+			XLogRegisterBufData(0, (char *) offsets,
+								presult->nfrozen * sizeof(OffsetNumber));
+
+			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+
+			PageSetLSN(page, recptr);
+		}
+
+		END_CRIT_SECTION();
 	}
 	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
 	{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index d14f36d9ce7..41ebbb9f931 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
 #ifndef HEAPAM_H
 #define HEAPAM_H
 
+#include "access/heapam_xlog.h"
 #include "access/relation.h"	/* for backward compatibility */
 #include "access/relscan.h"
 #include "access/sdir.h"
@@ -321,9 +322,16 @@ extern void heap_pre_freeze_checks(Buffer buffer,
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 										 TransactionId snapshotConflictHorizon,
 										 HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+										HeapTupleFreeze *tuples, int ntuples);
 extern bool heap_freeze_tuple(HeapTupleHeader tuple,
 							  TransactionId relfrozenxid, TransactionId relminmxid,
 							  TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
+extern int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
+								 xl_heap_freeze_plan *plans_out,
+								 OffsetNumber *offsets_out);
 extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
 									 const struct VacuumCutoffs *cutoffs,
 									 TransactionId *NoFreezePageRelfrozenXid,
-- 
2.40.1


--rdqtp5puvxqotfdw
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0011-Exit-heap_page_prune-early-if-no-prune.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

* [PATCH v3 10/17] Inline heap_freeze_execute_prepared()
@ 2024-01-07 22:03  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 7+ messages in thread

From: Melanie Plageman @ 2024-01-07 22:03 UTC (permalink / raw)

In order to merge freeze and prune records, the execution of tuple
freezing and the WAL logging of the changes to the page must be
separated so that the WAL logging can be combined with prune WAL
logging. This commit makes a helper for the tuple freezing and then
inlines the contents of heap_freeze_execute_prepared() where it is
called in heap_page_prune(). The original function,
heap_freeze_execute_prepared() is retained because the "no prune" case
in heap_page_prune() must still be able to emit a freeze record.
---
 src/backend/access/heap/heapam.c    | 61 +++++++++++++++++------------
 src/backend/access/heap/pruneheap.c | 51 ++++++++++++++++++++++--
 src/include/access/heapam.h         |  8 ++++
 3 files changed, 90 insertions(+), 30 deletions(-)

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 16e3f2520a4..a3691584c55 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -91,9 +91,6 @@ static void compute_new_xmax_infomask(TransactionId xmax, uint16 old_infomask,
 static TM_Result heap_lock_updated_tuple(Relation rel, HeapTuple tuple,
 										 ItemPointer ctid, TransactionId xid,
 										 LockTupleMode mode);
-static int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
-								 xl_heap_freeze_plan *plans_out,
-								 OffsetNumber *offsets_out);
 static void GetMultiXactIdHintBits(MultiXactId multi, uint16 *new_infomask,
 								   uint16 *new_infomask2);
 static TransactionId MultiXactIdGetUpdateXid(TransactionId xmax,
@@ -6713,30 +6710,17 @@ heap_pre_freeze_checks(Buffer buffer,
 }
 
 /*
- * heap_freeze_execute_prepared
- *
- * Executes freezing of one or more heap tuples on a page on behalf of caller.
- * Caller passes an array of tuple plans from heap_prepare_freeze_tuple.
- * Caller must set 'offset' in each plan for us.  Note that we destructively
- * sort caller's tuples array in-place, so caller had better be done with it.
- *
- * WAL-logs the changes so that VACUUM can advance the rel's relfrozenxid
- * later on without any risk of unsafe pg_xact lookups, even following a hard
- * crash (or when querying from a standby).  We represent freezing by setting
- * infomask bits in tuple headers, but this shouldn't be thought of as a hint.
- * See section on buffer access rules in src/backend/storage/buffer/README.
+ * Helper which executes freezing of one or more heap tuples on a page on
+ * behalf of caller. Caller passes an array of tuple plans from
+ * heap_prepare_freeze_tuple. Caller must set 'offset' in each plan for us.
+ * Must be called in a critical section that also marks the buffer dirty and,
+ * if needed, emits WAL.
  */
 void
-heap_freeze_execute_prepared(Relation rel, Buffer buffer,
-							 TransactionId snapshotConflictHorizon,
-							 HeapTupleFreeze *tuples, int ntuples)
+heap_freeze_prepared_tuples(Buffer buffer, HeapTupleFreeze *tuples, int ntuples)
 {
 	Page		page = BufferGetPage(buffer);
 
-	Assert(ntuples > 0);
-
-	START_CRIT_SECTION();
-
 	for (int i = 0; i < ntuples; i++)
 	{
 		HeapTupleFreeze *frz = tuples + i;
@@ -6746,6 +6730,29 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		htup = (HeapTupleHeader) PageGetItem(page, itemid);
 		heap_execute_freeze_tuple(htup, frz);
 	}
+}
+
+/*
+ * heap_freeze_execute_prepared
+ *
+ * Execute freezing of prepared tuples and WAL-logs the changes so that VACUUM
+ * can advance the rel's relfrozenxid later on without any risk of unsafe
+ * pg_xact lookups, even following a hard crash (or when querying from a
+ * standby).  We represent freezing by setting infomask bits in tuple headers,
+ * but this shouldn't be thought of as a hint. See section on buffer access
+ * rules in src/backend/storage/buffer/README. Must be called from within a
+ * critical section.
+ */
+void
+heap_freeze_execute_prepared(Relation rel, Buffer buffer,
+							 TransactionId snapshotConflictHorizon,
+							 HeapTupleFreeze *tuples, int ntuples)
+{
+	Page		page = BufferGetPage(buffer);
+
+	Assert(ntuples > 0);
+
+	heap_freeze_prepared_tuples(buffer, tuples, ntuples);
 
 	MarkBufferDirty(buffer);
 
@@ -6758,7 +6765,11 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 		xl_heap_freeze_page xlrec;
 		XLogRecPtr	recptr;
 
-		/* Prepare deduplicated representation for use in WAL record */
+		/*
+		 * Prepare deduplicated representation for use in WAL record
+		 * Destructively sorts tuples array in-place, so caller had better be
+		 * done with it.
+		 */
 		nplans = heap_log_freeze_plan(tuples, ntuples, plans, offsets);
 
 		xlrec.snapshotConflictHorizon = snapshotConflictHorizon;
@@ -6783,8 +6794,6 @@ heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 
 		PageSetLSN(page, recptr);
 	}
-
-	END_CRIT_SECTION();
 }
 
 /*
@@ -6874,7 +6883,7 @@ heap_log_freeze_new_plan(xl_heap_freeze_plan *plan, HeapTupleFreeze *frz)
  * (actually there is one array per freeze plan, but that's not of immediate
  * concern to our caller).
  */
-static int
+int
 heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
 					 xl_heap_freeze_plan *plans_out,
 					 OffsetNumber *offsets_out)
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bc0a23da61b..d4356e0bce9 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -627,10 +627,53 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 
 	if (do_freeze)
 	{
-		/* Execute all freeze plans for page as a single atomic action */
-		heap_freeze_execute_prepared(relation, buffer,
-									 frz_conflict_horizon,
-									 frozen, presult->nfrozen);
+		START_CRIT_SECTION();
+
+		Assert(presult->nfrozen > 0);
+
+		heap_freeze_prepared_tuples(buffer, frozen, presult->nfrozen);
+
+		MarkBufferDirty(buffer);
+
+		/* Now WAL-log freezing if necessary */
+		if (RelationNeedsWAL(relation))
+		{
+			xl_heap_freeze_plan plans[MaxHeapTuplesPerPage];
+			OffsetNumber offsets[MaxHeapTuplesPerPage];
+			int			nplans;
+			xl_heap_freeze_page xlrec;
+			XLogRecPtr	recptr;
+
+			/*
+			 * Prepare deduplicated representation for use in WAL record
+			 * Destructively sorts tuples array in-place.
+			 */
+			nplans = heap_log_freeze_plan(frozen, presult->nfrozen, plans, offsets);
+
+			xlrec.snapshotConflictHorizon = frz_conflict_horizon;
+			xlrec.isCatalogRel = RelationIsAccessibleInLogicalDecoding(relation);
+			xlrec.nplans = nplans;
+
+			XLogBeginInsert();
+			XLogRegisterData((char *) &xlrec, SizeOfHeapFreezePage);
+
+			/*
+			 * The freeze plan array and offset array are not actually in the
+			 * buffer, but pretend that they are.  When XLogInsert stores the
+			 * whole buffer, the arrays need not be stored too.
+			 */
+			XLogRegisterBuffer(0, buffer, REGBUF_STANDARD);
+			XLogRegisterBufData(0, (char *) plans,
+								nplans * sizeof(xl_heap_freeze_plan));
+			XLogRegisterBufData(0, (char *) offsets,
+								presult->nfrozen * sizeof(OffsetNumber));
+
+			recptr = XLogInsert(RM_HEAP2_ID, XLOG_HEAP2_FREEZE_PAGE);
+
+			PageSetLSN(page, recptr);
+		}
+
+		END_CRIT_SECTION();
 	}
 	else if (!pagefrz || !presult->all_frozen || presult->nfrozen > 0)
 	{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index dffbbd3cd3e..8a6bc071345 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -14,6 +14,7 @@
 #ifndef HEAPAM_H
 #define HEAPAM_H
 
+#include "access/heapam_xlog.h"
 #include "access/relation.h"	/* for backward compatibility */
 #include "access/relscan.h"
 #include "access/sdir.h"
@@ -320,9 +321,16 @@ extern void heap_pre_freeze_checks(Buffer buffer,
 extern void heap_freeze_execute_prepared(Relation rel, Buffer buffer,
 										 TransactionId snapshotConflictHorizon,
 										 HeapTupleFreeze *tuples, int ntuples);
+
+extern void heap_freeze_prepared_tuples(Buffer buffer,
+										HeapTupleFreeze *tuples, int ntuples);
 extern bool heap_freeze_tuple(HeapTupleHeader tuple,
 							  TransactionId relfrozenxid, TransactionId relminmxid,
 							  TransactionId FreezeLimit, TransactionId MultiXactCutoff);
+
+extern int	heap_log_freeze_plan(HeapTupleFreeze *tuples, int ntuples,
+								 xl_heap_freeze_plan *plans_out,
+								 OffsetNumber *offsets_out);
 extern bool heap_tuple_should_freeze(HeapTupleHeader tuple,
 									 const struct VacuumCutoffs *cutoffs,
 									 TransactionId *NoFreezePageRelfrozenXid,
-- 
2.40.1


--racicctn4wry6xe5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0011-Exit-heap_page_prune-early-if-no-prune.patch"



^ permalink  raw  reply  [nested|flat] 7+ messages in thread

end of thread, other threads:[~2024-01-07 22:03 UTC | newest]

Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-04-20 15:49 Re: Wrong results from Parallel Hash Full Join Melanie Plageman <[email protected]>
2024-01-07 22:03 [PATCH v2 10/17] Inline heap_freeze_execute_prepared() Melanie Plageman <[email protected]>
2024-01-07 22:03 [PATCH v3 10/17] Inline heap_freeze_execute_prepared() Melanie Plageman <[email protected]>
2024-01-07 22:03 [PATCH v2 10/17] Inline heap_freeze_execute_prepared() Melanie Plageman <[email protected]>
2024-01-07 22:03 [PATCH v2 10/17] Inline heap_freeze_execute_prepared() Melanie Plageman <[email protected]>
2024-01-07 22:03 [PATCH v3 10/17] Inline heap_freeze_execute_prepared() Melanie Plageman <[email protected]>
2024-01-07 22:03 [PATCH v3 10/17] Inline heap_freeze_execute_prepared() Melanie Plageman <[email protected]>

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