agora inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
Subject: [PATCH v1 04/14] heapam: Use exclusive lock on old page in CLUSTER
Date: Thu, 17 Oct 2024 11:45:36 -0400
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 coulda void 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.
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 +++++++
src/backend/storage/buffer/bufmgr.c | 2 +-
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index a8d95e0f1c1..009601445aa 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -833,7 +833,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 057b88767db..b7aa8bb7a52 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(BufferIsLocal(buffer) || BufferLockHeldByMe(buffer, BUFFER_LOCK_EXCLUSIVE));
+
SetHintBits(tuple, buffer, infomask, xid);
}
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 88d18e85d64..f56f7f9f441 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -5527,7 +5527,7 @@ BufferLockHeldByMe(Buffer buffer, int mode)
{
Assert(false);
pg_unreachable();
- lwmode = LW_EXCLUSIVE; /* assuage compiler */
+ lwmode = LW_EXCLUSIVE; /* assuage compiler */
}
return LWLockHeldByMeInMode(BufferDescriptorGetContentLock(buf), lwmode);
--
2.46.0.519.g2e7b89e038
--gpbnw6zzq3spol54
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v1-0005-heapam-Only-set-tuple-s-block-once-per-page-in-pa.patch"
view thread (31+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v1 04/14] heapam: Use exclusive lock on old page in CLUSTER
In-Reply-To: <no-message-id-55463@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