Unpin buffer before inplace update waits for an XID to end.

Commit a07e03fd8f changed inplace updates
to wait for heap_update() commands like GRANT TABLE and GRANT DATABASE.
By keeping the pin during that wait, a sequence of autovacuum workers
and an uncommitted GRANT starved one foreground LockBufferForCleanup()
for six minutes, on buildfarm member sarus.  Prevent, at the cost of a
bit of complexity.  Back-patch to v12, like the earlier commit.  That
commit and heap_inplace_lock() have not yet appeared in any release.

Discussion: https://postgr.es/m/20241026184936.ae.nmisch@google.com
This commit is contained in:
Noah Misch 2024-10-29 09:39:55 -07:00
parent a0c8d600bb
commit 370bc77402
3 changed files with 14 additions and 11 deletions

View File

@ -6023,8 +6023,8 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
* transaction. If compatible, return true with the buffer exclusive-locked, * transaction. If compatible, return true with the buffer exclusive-locked,
* and the caller must release that by calling * and the caller must release that by calling
* heap_inplace_update_and_unlock(), calling heap_inplace_unlock(), or raising * heap_inplace_update_and_unlock(), calling heap_inplace_unlock(), or raising
* an error. Otherwise, return false after blocking transactions, if any, * an error. Otherwise, call release_callback(arg), wait for blocking
* have ended. * transactions to end, and return false.
* *
* Since this is intended for system catalogs and SERIALIZABLE doesn't cover * Since this is intended for system catalogs and SERIALIZABLE doesn't cover
* DDL, this doesn't guarantee any particular predicate locking. * DDL, this doesn't guarantee any particular predicate locking.
@ -6058,7 +6058,8 @@ heap_abort_speculative(Relation relation, ItemPointer tid)
*/ */
bool bool
heap_inplace_lock(Relation relation, heap_inplace_lock(Relation relation,
HeapTuple oldtup_ptr, Buffer buffer) HeapTuple oldtup_ptr, Buffer buffer,
void (*release_callback) (void *), void *arg)
{ {
HeapTupleData oldtup = *oldtup_ptr; /* minimize diff vs. heap_update() */ HeapTupleData oldtup = *oldtup_ptr; /* minimize diff vs. heap_update() */
TM_Result result; TM_Result result;
@ -6123,6 +6124,7 @@ heap_inplace_lock(Relation relation,
lockmode, NULL)) lockmode, NULL))
{ {
LockBuffer(buffer, BUFFER_LOCK_UNLOCK); LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
release_callback(arg);
ret = false; ret = false;
MultiXactIdWait((MultiXactId) xwait, mxact_status, infomask, MultiXactIdWait((MultiXactId) xwait, mxact_status, infomask,
relation, &oldtup.t_self, XLTW_Update, relation, &oldtup.t_self, XLTW_Update,
@ -6138,6 +6140,7 @@ heap_inplace_lock(Relation relation,
else else
{ {
LockBuffer(buffer, BUFFER_LOCK_UNLOCK); LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
release_callback(arg);
ret = false; ret = false;
XactLockTableWait(xwait, relation, &oldtup.t_self, XactLockTableWait(xwait, relation, &oldtup.t_self,
XLTW_Update); XLTW_Update);
@ -6149,6 +6152,7 @@ heap_inplace_lock(Relation relation,
if (!ret) if (!ret)
{ {
LockBuffer(buffer, BUFFER_LOCK_UNLOCK); LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
release_callback(arg);
} }
} }

View File

@ -805,6 +805,7 @@ systable_inplace_update_begin(Relation relation,
int retries = 0; int retries = 0;
SysScanDesc scan; SysScanDesc scan;
HeapTuple oldtup; HeapTuple oldtup;
BufferHeapTupleTableSlot *bslot;
/* /*
* For now, we don't allow parallel updates. Unlike a regular update, * For now, we don't allow parallel updates. Unlike a regular update,
@ -826,10 +827,9 @@ systable_inplace_update_begin(Relation relation,
Assert(IsInplaceUpdateRelation(relation) || !IsSystemRelation(relation)); Assert(IsInplaceUpdateRelation(relation) || !IsSystemRelation(relation));
/* Loop for an exclusive-locked buffer of a non-updated tuple. */ /* Loop for an exclusive-locked buffer of a non-updated tuple. */
for (;;) do
{ {
TupleTableSlot *slot; TupleTableSlot *slot;
BufferHeapTupleTableSlot *bslot;
CHECK_FOR_INTERRUPTS(); CHECK_FOR_INTERRUPTS();
@ -855,11 +855,9 @@ systable_inplace_update_begin(Relation relation,
slot = scan->slot; slot = scan->slot;
Assert(TTS_IS_BUFFERTUPLE(slot)); Assert(TTS_IS_BUFFERTUPLE(slot));
bslot = (BufferHeapTupleTableSlot *) slot; bslot = (BufferHeapTupleTableSlot *) slot;
if (heap_inplace_lock(scan->heap_rel, } while (!heap_inplace_lock(scan->heap_rel,
bslot->base.tuple, bslot->buffer)) bslot->base.tuple, bslot->buffer,
break; (void (*) (void *)) systable_endscan, scan));
systable_endscan(scan);
};
*oldtupcopy = heap_copytuple(oldtup); *oldtupcopy = heap_copytuple(oldtup);
*state = scan; *state = scan;

View File

@ -257,7 +257,8 @@ extern TM_Result heap_lock_tuple(Relation relation, HeapTuple tuple,
Buffer *buffer, struct TM_FailureData *tmfd); Buffer *buffer, struct TM_FailureData *tmfd);
extern bool heap_inplace_lock(Relation relation, extern bool heap_inplace_lock(Relation relation,
HeapTuple oldtup_ptr, Buffer buffer); HeapTuple oldtup_ptr, Buffer buffer,
void (*release_callback) (void *), void *arg);
extern void heap_inplace_update_and_unlock(Relation relation, extern void heap_inplace_update_and_unlock(Relation relation,
HeapTuple oldtup, HeapTuple tuple, HeapTuple oldtup, HeapTuple tuple,
Buffer buffer); Buffer buffer);