From 727f753b56640fff91a2bfcdbf58ea4edbd5b65e Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 28 Oct 2024 16:44:11 -0400 Subject: [PATCH v2 11/15] Acquire right to set hint bits in the remaining places Use BufferPrepareToSetHintBits() in the remaining places that modify buffers without an exclusive lock. The remaining places are indexes with support for kill_prior_tuples and the freespace code. After this we do not need to copy buffers to write them out anymore. That change is done separately however. Author: Reviewed-by: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/access/gist/gistget.c | 15 +++++++----- src/backend/access/hash/hashutil.c | 6 ++++- src/backend/access/nbtree/nbtinsert.c | 29 +++++++++++++++-------- src/backend/access/nbtree/nbtutils.c | 7 +++++- src/backend/storage/freespace/freespace.c | 16 +++++++++---- src/backend/storage/freespace/fsmpage.c | 8 ++++++- 6 files changed, 57 insertions(+), 24 deletions(-) diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c index b35b8a97577..020993b309c 100644 --- a/src/backend/access/gist/gistget.c +++ b/src/backend/access/gist/gistget.c @@ -63,11 +63,7 @@ gistkillitems(IndexScanDesc scan) * safe. */ if (BufferGetLSNAtomic(buffer) != so->curPageLSN) - { - UnlockReleaseBuffer(buffer); - so->numKilled = 0; /* reset counter */ - return; - } + goto unlock; Assert(GistPageIsLeaf(page)); @@ -77,6 +73,12 @@ gistkillitems(IndexScanDesc scan) */ for (i = 0; i < so->numKilled; i++) { + if (!killedsomething) + { + if (!BufferPrepareToSetHintBits(buffer)) + goto unlock; + } + offnum = so->killedItems[i]; iid = PageGetItemId(page, offnum); ItemIdMarkDead(iid); @@ -86,9 +88,10 @@ gistkillitems(IndexScanDesc scan) if (killedsomething) { GistMarkPageHasGarbage(page); - MarkBufferDirtyHint(buffer, true); + BufferFinishSetHintBits(buffer, true, true); } +unlock: UnlockReleaseBuffer(buffer); /* diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c index 20028f5cd14..a7fc98d8f96 100644 --- a/src/backend/access/hash/hashutil.c +++ b/src/backend/access/hash/hashutil.c @@ -593,6 +593,9 @@ _hash_kill_items(IndexScanDesc scan) if (ItemPointerEquals(&ituple->t_tid, &currItem->heapTid)) { + if (!BufferPrepareToSetHintBits(so->currPos.buf)) + goto unlock_page; + /* found the item */ ItemIdMarkDead(iid); killedsomething = true; @@ -610,9 +613,10 @@ _hash_kill_items(IndexScanDesc scan) if (killedsomething) { opaque->hasho_flag |= LH_PAGE_HAS_DEAD_TUPLES; - MarkBufferDirtyHint(buf, true); + BufferFinishSetHintBits(so->currPos.buf, true, true); } +unlock_page: if (so->hashso_bucket_buf == so->currPos.buf || havePin) LockBuffer(so->currPos.buf, BUFFER_LOCK_UNLOCK); diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c index 99043da8412..75160d53e59 100644 --- a/src/backend/access/nbtree/nbtinsert.c +++ b/src/backend/access/nbtree/nbtinsert.c @@ -679,20 +679,29 @@ _bt_check_unique(Relation rel, BTInsertState insertstate, Relation heapRel, { /* * The conflicting tuple (or all HOT chains pointed to by - * all posting list TIDs) is dead to everyone, so mark the - * index entry killed. + * all posting list TIDs) is dead to everyone, so try to + * mark the index entry killed. It's ok if we're not + * allowed to, this isn't required for correctness. */ - ItemIdMarkDead(curitemid); - opaque->btpo_flags |= BTP_HAS_GARBAGE; + Buffer buf; - /* - * Mark buffer with a dirty hint, since state is not - * crucial. Be sure to mark the proper buffer dirty. - */ + /* Be sure to operate on the proper buffer */ if (nbuf != InvalidBuffer) - MarkBufferDirtyHint(nbuf, true); + buf = nbuf; else - MarkBufferDirtyHint(insertstate->buf, true); + buf = insertstate->buf; + + if (BufferPrepareToSetHintBits(buf)) + { + ItemIdMarkDead(curitemid); + opaque->btpo_flags |= BTP_HAS_GARBAGE; + + /* + * Mark buffer with a dirty hint, since state is not + * crucial. + */ + BufferFinishSetHintBits(buf, true, true); + } } /* diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index d7603250279..417b7c11d6f 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -4318,6 +4318,10 @@ _bt_killitems(IndexScanDesc scan) */ if (killtuple && !ItemIdIsDead(iid)) { + /* if we're not able to set hint bits, there's no point continuing */ + if (!killedsomething && !BufferPrepareToSetHintBits(so->currPos.buf)) + goto unlock_page; + /* found the item/all posting list items */ ItemIdMarkDead(iid); killedsomething = true; @@ -4337,9 +4341,10 @@ _bt_killitems(IndexScanDesc scan) if (killedsomething) { opaque->btpo_flags |= BTP_HAS_GARBAGE; - MarkBufferDirtyHint(so->currPos.buf, true); + BufferFinishSetHintBits(so->currPos.buf, true, true); } +unlock_page: _bt_unlockbuf(scan->indexRelation, so->currPos.buf); } diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c index b1262ac3b69..90b306a65cd 100644 --- a/src/backend/storage/freespace/freespace.c +++ b/src/backend/storage/freespace/freespace.c @@ -904,12 +904,18 @@ fsm_vacuum_page(Relation rel, FSMAddress addr, max_avail = fsm_get_max_avail(page); /* - * Reset the next slot pointer. This encourages the use of low-numbered - * pages, increasing the chances that a later vacuum can truncate the - * relation. We don't bother with a lock here, nor with marking the page - * dirty if it wasn't already, since this is just a hint. + * Try to reset the next slot pointer. This encourages the use of + * low-numbered pages, increasing the chances that a later vacuum can + * truncate the relation. We don't bother with a lock here, nor with + * marking the page dirty if it wasn't already, since this is just a hint. + * + * FIXME: Should probably use BufferSetHintBits16() */ - ((FSMPage) PageGetContents(page))->fp_next_slot = 0; + if (BufferPrepareToSetHintBits(buf)) + { + ((FSMPage) PageGetContents(page))->fp_next_slot = 0; + BufferFinishSetHintBits(buf, false, false); + } ReleaseBuffer(buf); diff --git a/src/backend/storage/freespace/fsmpage.c b/src/backend/storage/freespace/fsmpage.c index ba85cad0697..257d4ed12c0 100644 --- a/src/backend/storage/freespace/fsmpage.c +++ b/src/backend/storage/freespace/fsmpage.c @@ -300,7 +300,13 @@ restart: * * Wrap-around is handled at the beginning of this function. */ - fsmpage->fp_next_slot = slot + (advancenext ? 1 : 0); + if (exclusive_lock_held || BufferPrepareToSetHintBits(buf)) + { + fsmpage->fp_next_slot = slot + (advancenext ? 1 : 0); + + if (!exclusive_lock_held) + BufferFinishSetHintBits(buf, false, true); + } return slot; } -- 2.45.2.746.g06e570c0df.dirty