From 853d9e57765e89db711ac6d9751061a2327a571f Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 28 Oct 2024 16:44:11 -0400 Subject: [PATCH v1 10/14] 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 | 12 +++++---- src/backend/access/hash/hashutil.c | 6 +++++ src/backend/access/nbtree/nbtinsert.c | 31 +++++++++++++++-------- src/backend/access/nbtree/nbtutils.c | 7 +++++ src/backend/storage/freespace/freespace.c | 14 ++++++---- src/backend/storage/freespace/fsmpage.c | 8 +++++- 6 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/backend/access/gist/gistget.c b/src/backend/access/gist/gistget.c index b35b8a97577..c0a39768fdf 100644 --- a/src/backend/access/gist/gistget.c +++ b/src/backend/access/gist/gistget.c @@ -57,17 +57,16 @@ gistkillitems(IndexScanDesc scan) gistcheckpage(scan->indexRelation, buffer); page = BufferGetPage(buffer); + if (!BufferPrepareToSetHintBits(buffer)) + goto unlock; + /* * If page LSN differs it means that the page was modified since the last * read. killedItems could be not valid so LP_DEAD hints applying is not * safe. */ if (BufferGetLSNAtomic(buffer) != so->curPageLSN) - { - UnlockReleaseBuffer(buffer); - so->numKilled = 0; /* reset counter */ - return; - } + goto unlock; Assert(GistPageIsLeaf(page)); @@ -89,6 +88,9 @@ gistkillitems(IndexScanDesc scan) MarkBufferDirtyHint(buffer, true); } + BufferFinishSetHintBits(buffer); + +unlock: UnlockReleaseBuffer(buffer); /* diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c index 20028f5cd14..a9adf95bafa 100644 --- a/src/backend/access/hash/hashutil.c +++ b/src/backend/access/hash/hashutil.c @@ -572,6 +572,9 @@ _hash_kill_items(IndexScanDesc scan) else buf = _hash_getbuf(rel, blkno, HASH_READ, LH_OVERFLOW_PAGE); + if (!BufferPrepareToSetHintBits(so->currPos.buf)) + goto unlock_page; + page = BufferGetPage(buf); opaque = HashPageGetOpaque(page); maxoff = PageGetMaxOffsetNumber(page); @@ -613,6 +616,9 @@ _hash_kill_items(IndexScanDesc scan) MarkBufferDirtyHint(buf, true); } + BufferFinishSetHintBits(so->currPos.buf); + +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..eef498ada7c 100644 --- a/src/backend/access/nbtree/nbtinsert.c +++ b/src/backend/access/nbtree/nbtinsert.c @@ -679,20 +679,31 @@ _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. + */ + MarkBufferDirtyHint(buf, true); + + BufferFinishSetHintBits(buf); + } } /* diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c index 76be65123c8..ea618a5565d 100644 --- a/src/backend/access/nbtree/nbtutils.c +++ b/src/backend/access/nbtree/nbtutils.c @@ -4210,6 +4210,10 @@ _bt_killitems(IndexScanDesc scan) } } + /* if we're not able to set hint bits, there's no point continuing */ + if (!BufferPrepareToSetHintBits(so->currPos.buf)) + goto unlock_page; + opaque = BTPageGetOpaque(page); minoff = P_FIRSTDATAKEY(opaque); maxoff = PageGetMaxOffsetNumber(page); @@ -4329,6 +4333,9 @@ _bt_killitems(IndexScanDesc scan) MarkBufferDirtyHint(so->currPos.buf, true); } + BufferFinishSetHintBits(so->currPos.buf); + +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..864c7e231ee 100644 --- a/src/backend/storage/freespace/freespace.c +++ b/src/backend/storage/freespace/freespace.c @@ -904,12 +904,16 @@ 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. */ - ((FSMPage) PageGetContents(page))->fp_next_slot = 0; + if (BufferPrepareToSetHintBits(buf)) + { + ((FSMPage) PageGetContents(page))->fp_next_slot = 0; + BufferFinishSetHintBits(buf); + } ReleaseBuffer(buf); diff --git a/src/backend/storage/freespace/fsmpage.c b/src/backend/storage/freespace/fsmpage.c index ba85cad0697..84bfa410dc0 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); + } return slot; } -- 2.46.0.519.g2e7b89e038