Convert many uses of ReadBuffer[Extended](P_NEW) to ExtendBufferedRel()

A few places are not converted. Some because they are tackled in later
commits (e.g. hio.c, xlogutils.c), some because they are more
complicated (e.g. brin_pageops.c).  Having a few users of ReadBuffer(P_NEW) is
good anyway, to ensure the backward compat path stays working.

Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de
This commit is contained in:
Andres Freund 2023-04-05 18:57:29 -07:00
parent fcdda1e4b5
commit acab1b0914
13 changed files with 42 additions and 96 deletions

View File

@ -353,7 +353,6 @@ Buffer
BloomNewBuffer(Relation index)
{
Buffer buffer;
bool needLock;
/* First, try to get a page from FSM */
for (;;)
@ -387,15 +386,8 @@ BloomNewBuffer(Relation index)
}
/* Must extend the file */
needLock = !RELATION_IS_LOCAL(index);
if (needLock)
LockRelationForExtension(index, ExclusiveLock);
buffer = ReadBuffer(index, P_NEW);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
if (needLock)
UnlockRelationForExtension(index, ExclusiveLock);
buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL,
EB_LOCK_FIRST);
return buffer;
}

View File

@ -837,9 +837,9 @@ brinbuild(Relation heap, Relation index, IndexInfo *indexInfo)
* whole relation will be rolled back.
*/
meta = ReadBuffer(index, P_NEW);
meta = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL,
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
Assert(BufferGetBlockNumber(meta) == BRIN_METAPAGE_BLKNO);
LockBuffer(meta, BUFFER_LOCK_EXCLUSIVE);
brin_metapage_init(BufferGetPage(meta), BrinGetPagesPerRange(index),
BRIN_CURRENT_VERSION);
@ -904,9 +904,8 @@ brinbuildempty(Relation index)
Buffer metabuf;
/* An empty BRIN index has a metapage only. */
metabuf =
ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
LockBuffer(metabuf, BUFFER_LOCK_EXCLUSIVE);
metabuf = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL,
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
/* Initialize and xlog metabuffer. */
START_CRIT_SECTION();

View File

@ -730,6 +730,10 @@ brin_getinsertbuffer(Relation irel, Buffer oldbuf, Size itemsz,
* There's not enough free space in any existing index page,
* according to the FSM: extend the relation to obtain a shiny new
* page.
*
* XXX: It's likely possible to use RBM_ZERO_AND_LOCK here,
* which'd avoid the need to hold the extension lock during buffer
* reclaim.
*/
if (!RELATION_IS_LOCAL(irel))
{

View File

@ -538,7 +538,6 @@ revmap_physical_extend(BrinRevmap *revmap)
BlockNumber mapBlk;
BlockNumber nblocks;
Relation irel = revmap->rm_irel;
bool needLock = !RELATION_IS_LOCAL(irel);
/*
* Lock the metapage. This locks out concurrent extensions of the revmap,
@ -570,10 +569,8 @@ revmap_physical_extend(BrinRevmap *revmap)
}
else
{
if (needLock)
LockRelationForExtension(irel, ExclusiveLock);
buf = ReadBuffer(irel, P_NEW);
buf = ExtendBufferedRel(EB_REL(irel), MAIN_FORKNUM, NULL,
EB_LOCK_FIRST);
if (BufferGetBlockNumber(buf) != mapBlk)
{
/*
@ -582,17 +579,11 @@ revmap_physical_extend(BrinRevmap *revmap)
* up and have caller start over. We will have to evacuate that
* page from under whoever is using it.
*/
if (needLock)
UnlockRelationForExtension(irel, ExclusiveLock);
LockBuffer(revmap->rm_metaBuf, BUFFER_LOCK_UNLOCK);
ReleaseBuffer(buf);
UnlockReleaseBuffer(buf);
return;
}
LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
page = BufferGetPage(buf);
if (needLock)
UnlockRelationForExtension(irel, ExclusiveLock);
}
/* Check that it's a regular block (or an empty page) */

View File

@ -440,12 +440,10 @@ ginbuildempty(Relation index)
MetaBuffer;
/* An empty GIN index has two pages. */
MetaBuffer =
ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
LockBuffer(MetaBuffer, BUFFER_LOCK_EXCLUSIVE);
RootBuffer =
ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
LockBuffer(RootBuffer, BUFFER_LOCK_EXCLUSIVE);
MetaBuffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL,
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
RootBuffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL,
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
/* Initialize and xlog metabuffer and root buffer. */
START_CRIT_SECTION();

View File

@ -299,7 +299,6 @@ Buffer
GinNewBuffer(Relation index)
{
Buffer buffer;
bool needLock;
/* First, try to get a page from FSM */
for (;;)
@ -328,15 +327,8 @@ GinNewBuffer(Relation index)
}
/* Must extend the file */
needLock = !RELATION_IS_LOCAL(index);
if (needLock)
LockRelationForExtension(index, ExclusiveLock);
buffer = ReadBuffer(index, P_NEW);
LockBuffer(buffer, GIN_EXCLUSIVE);
if (needLock)
UnlockRelationForExtension(index, ExclusiveLock);
buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL,
EB_LOCK_FIRST);
return buffer;
}

View File

@ -134,8 +134,8 @@ gistbuildempty(Relation index)
Buffer buffer;
/* Initialize the root page */
buffer = ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_NORMAL, NULL);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
buffer = ExtendBufferedRel(EB_REL(index), INIT_FORKNUM, NULL,
EB_SKIP_EXTENSION_LOCK | EB_LOCK_FIRST);
/* Initialize and xlog buffer */
START_CRIT_SECTION();

View File

@ -824,7 +824,6 @@ Buffer
gistNewBuffer(Relation r, Relation heaprel)
{
Buffer buffer;
bool needLock;
/* First, try to get a page from FSM */
for (;;)
@ -878,16 +877,8 @@ gistNewBuffer(Relation r, Relation heaprel)
}
/* Must extend the file */
needLock = !RELATION_IS_LOCAL(r);
if (needLock)
LockRelationForExtension(r, ExclusiveLock);
buffer = ReadBuffer(r, P_NEW);
LockBuffer(buffer, GIST_EXCLUSIVE);
if (needLock)
UnlockRelationForExtension(r, ExclusiveLock);
buffer = ExtendBufferedRel(EB_REL(r), MAIN_FORKNUM, NULL,
EB_LOCK_FIRST);
return buffer;
}

View File

@ -206,14 +206,14 @@ _hash_getnewbuf(Relation rel, BlockNumber blkno, ForkNumber forkNum)
elog(ERROR, "access to noncontiguous page in hash index \"%s\"",
RelationGetRelationName(rel));
/* smgr insists we use P_NEW to extend the relation */
/* smgr insists we explicitly extend the relation */
if (blkno == nblocks)
{
buf = ReadBufferExtended(rel, forkNum, P_NEW, RBM_NORMAL, NULL);
buf = ExtendBufferedRel(EB_REL(rel), forkNum, NULL,
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
if (BufferGetBlockNumber(buf) != blkno)
elog(ERROR, "unexpected hash relation size: %u, should be %u",
BufferGetBlockNumber(buf), blkno);
LockBuffer(buf, HASH_WRITE);
}
else
{

View File

@ -882,7 +882,6 @@ _bt_getbuf(Relation rel, Relation heaprel, BlockNumber blkno, int access)
}
else
{
bool needLock;
Page page;
Assert(access == BT_WRITE);
@ -963,31 +962,16 @@ _bt_getbuf(Relation rel, Relation heaprel, BlockNumber blkno, int access)
}
/*
* Extend the relation by one page.
*
* We have to use a lock to ensure no one else is extending the rel at
* the same time, else we will both try to initialize the same new
* page. We can skip locking for new or temp relations, however,
* since no one else could be accessing them.
* Extend the relation by one page. Need to use RBM_ZERO_AND_LOCK or
* we risk a race condition against btvacuumscan --- see comments
* therein. This forces us to repeat the valgrind request that
* _bt_lockbuf() otherwise would make, as we can't use _bt_lockbuf()
* without introducing a race.
*/
needLock = !RELATION_IS_LOCAL(rel);
if (needLock)
LockRelationForExtension(rel, ExclusiveLock);
buf = ReadBuffer(rel, P_NEW);
/* Acquire buffer lock on new page */
_bt_lockbuf(rel, buf, BT_WRITE);
/*
* Release the file-extension lock; it's now OK for someone else to
* extend the relation some more. Note that we cannot release this
* lock before we have buffer lock on the new page, or we risk a race
* condition against btvacuumscan --- see comments therein.
*/
if (needLock)
UnlockRelationForExtension(rel, ExclusiveLock);
buf = ExtendBufferedRel(EB_REL(rel), MAIN_FORKNUM, NULL,
EB_LOCK_FIRST);
if (!RelationUsesLocalBuffers(rel))
VALGRIND_MAKE_MEM_DEFINED(BufferGetPage(buf), BLCKSZ);
/* Initialize the new page before returning it */
page = BufferGetPage(buf);

View File

@ -970,6 +970,9 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
* write-lock on the left page before it adds a right page, so we must
* already have processed any tuples due to be moved into such a page.
*
* XXX: Now that new pages are locked with RBM_ZERO_AND_LOCK, I don't
* think the use of the extension lock is still required.
*
* We can skip locking for new or temp relations, however, since no one
* else could be accessing them.
*/

View File

@ -366,7 +366,6 @@ Buffer
SpGistNewBuffer(Relation index)
{
Buffer buffer;
bool needLock;
/* First, try to get a page from FSM */
for (;;)
@ -406,16 +405,8 @@ SpGistNewBuffer(Relation index)
ReleaseBuffer(buffer);
}
/* Must extend the file */
needLock = !RELATION_IS_LOCAL(index);
if (needLock)
LockRelationForExtension(index, ExclusiveLock);
buffer = ReadBuffer(index, P_NEW);
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
if (needLock)
UnlockRelationForExtension(index, ExclusiveLock);
buffer = ExtendBufferedRel(EB_REL(index), MAIN_FORKNUM, NULL,
EB_LOCK_FIRST);
return buffer;
}

View File

@ -377,7 +377,8 @@ fill_seq_fork_with_data(Relation rel, HeapTuple tuple, ForkNumber forkNum)
/* Initialize first page of relation with special magic number */
buf = ReadBufferExtended(rel, forkNum, P_NEW, RBM_ZERO_AND_LOCK, NULL);
buf = ExtendBufferedRel(EB_REL(rel), forkNum, NULL,
EB_LOCK_FIRST | EB_SKIP_EXTENSION_LOCK);
Assert(BufferGetBlockNumber(buf) == 0);
page = BufferGetPage(buf);