agora inbox for [email protected]  
help / color / mirror / Atom feed
From: Andres Freund <[email protected]>
Subject: [PATCH v8 04/10] heapam: Use exclusive lock on old page in CLUSTER
Date: Sun, 26 Jan 2025 15:18:46 -0500

To be able to guarantee that we can set the hint bit, acquire an exclusive
lock on the old buffer. We need the hint bits to be set as otherwise
reform_and_rewrite_tuple() -> rewrite_heap_tuple() -> heap_freeze_tuple() will
get confused.

It'd be better if we somehow could avoid setting hint bits on the old page. A
commonreason to use VACUUM FULL are very bloated tables - rewriting most of
the old table before during VACUUM FULL doesn't exactly help.

Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/access/heap/heapam_handler.c    | 13 ++++++++++++-
 src/backend/access/heap/heapam_visibility.c |  7 +++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index dd4fe6bf62f..0c684786382 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -837,7 +837,18 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
 		tuple = ExecFetchSlotHeapTuple(slot, false, NULL);
 		buf = hslot->buffer;
 
-		LockBuffer(buf, BUFFER_LOCK_SHARE);
+		/*
+		 * To be able to guarantee that we can set the hint bit, acquire an
+		 * exclusive lock on the old buffer. We need the hint bits to be set
+		 * as otherwise reform_and_rewrite_tuple() -> rewrite_heap_tuple() ->
+		 * heap_freeze_tuple() will get confused.
+		 *
+		 * It'd be better if we somehow could avoid setting hint bits on the
+		 * old page. One reason to use VACUUM FULL are very bloated tables -
+		 * rewriting most of the old table before during VACUUM FULL doesn't
+		 * exactly help...
+		 */
+		LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
 
 		switch (HeapTupleSatisfiesVacuum(tuple, OldestXmin, buf))
 		{
diff --git a/src/backend/access/heap/heapam_visibility.c b/src/backend/access/heap/heapam_visibility.c
index bf899c2d2c6..debf5d56b95 100644
--- a/src/backend/access/heap/heapam_visibility.c
+++ b/src/backend/access/heap/heapam_visibility.c
@@ -141,6 +141,13 @@ void
 HeapTupleSetHintBits(HeapTupleHeader tuple, Buffer buffer,
 					 uint16 infomask, TransactionId xid)
 {
+	/*
+	 * The uses from heapam.c rely on being able to perform the hint bit
+	 * updates, which can only be guaranteed if we are holding an exclusive
+	 * lock on the buffer - which all callers are doing.
+	 */
+	Assert(BufferIsLockedByMeInMode(buffer, BUFFER_LOCK_EXCLUSIVE));
+
 	SetHintBits(tuple, buffer, infomask, xid);
 }
 
-- 
2.48.1.76.g4e746b1a31.dirty


--lbbj3oq7cjvyhmbk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v8-0005-heapam-Add-batch-mode-mvcc-check-and-use-it-in-pa.patch"



view thread (31+ messages)

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]
  Subject: Re: [PATCH v8 04/10] heapam: Use exclusive lock on old page in CLUSTER
  In-Reply-To: <no-message-id-34006@localhost>

* 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