diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 09a456e9966..cbef73e5d4b 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -837,7 +837,21 @@ 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, set in + * heapam_relation_copy_for_cluster() -> HeapTupleSatisfiesVacuum(), + * to be set, as otherwise reform_and_rewrite_tuple() -> + * rewrite_heap_tuple() will get confused. Specifically, + * rewrite_heap_tuple() checks for HEAP_XMAX_INVALID in the old tuple + * to determine whether to check the old-to-new mapping hash table. + * + * 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 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 05e70b7d92a..9a034d5c9e8 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); } diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c index bae3a2da77a..77fd48eb59e 100644 --- a/src/backend/access/heap/rewriteheap.c +++ b/src/backend/access/heap/rewriteheap.c @@ -382,6 +382,9 @@ rewrite_heap_tuple(RewriteState state, /* * If the tuple has been updated, check the old-to-new mapping hash table. + * + * Note that this check relies on the HeapTupleSatisfiesVacuum() in + * heapam_relation_copy_for_cluster() to have set hint bits. */ if (!((old_tuple->t_data->t_infomask & HEAP_XMAX_INVALID) || HeapTupleHeaderIsOnlyLocked(old_tuple->t_data)) &&