Skip to content

Commit 2361288

Browse files
anarazelCommitfest Bot
authored andcommitted
heapam: Use exclusive lock on old page in CLUSTER
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:
1 parent 4a0585d commit 2361288

File tree

3 files changed

+20
-2
lines changed

3 files changed

+20
-2
lines changed

src/backend/access/heap/heapam_handler.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -837,7 +837,18 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
837837
tuple = ExecFetchSlotHeapTuple(slot, false, NULL);
838838
buf = hslot->buffer;
839839

840-
LockBuffer(buf, BUFFER_LOCK_SHARE);
840+
/*
841+
* To be able to guarantee that we can set the hint bit, acquire an
842+
* exclusive lock on the old buffer. We need the hint bits to be set
843+
* as otherwise reform_and_rewrite_tuple() -> rewrite_heap_tuple() ->
844+
* heap_freeze_tuple() will get confused.
845+
*
846+
* It'd be better if we somehow could avoid setting hint bits on the
847+
* old page. One reason to use VACUUM FULL are very bloated tables -
848+
* rewriting most of the old table before during VACUUM FULL doesn't
849+
* exactly help...
850+
*/
851+
LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
841852

842853
switch (HeapTupleSatisfiesVacuum(tuple, OldestXmin, buf))
843854
{

src/backend/access/heap/heapam_visibility.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,13 @@ void
141141
HeapTupleSetHintBits(HeapTupleHeader tuple, Buffer buffer,
142142
uint16 infomask, TransactionId xid)
143143
{
144+
/*
145+
* The uses from heapam.c rely on being able to perform the hint bit
146+
* updates, which can only be guaranteed if we are holding an exclusive
147+
* lock on the buffer - which all callers are doing.
148+
*/
149+
Assert(BufferIsLocal(buffer) || BufferLockHeldByMe(buffer, BUFFER_LOCK_EXCLUSIVE));
150+
144151
SetHintBits(tuple, buffer, infomask, xid);
145152
}
146153

src/backend/storage/buffer/bufmgr.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5908,7 +5908,7 @@ BufferLockHeldByMe(Buffer buffer, int mode)
59085908
{
59095909
Assert(false);
59105910
pg_unreachable();
5911-
lwmode = LW_EXCLUSIVE; /* assuage compiler */
5911+
lwmode = LW_EXCLUSIVE; /* assuage compiler */
59125912
}
59135913

59145914
return LWLockHeldByMeInMode(BufferDescriptorGetContentLock(buf), lwmode);

0 commit comments

Comments
 (0)