agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedpg_migrator 8.4.1 alpha 1 released with bug mention
6+ messages / 2 participants
[nested] [flat]
* pg_migrator 8.4.1 alpha 1 released with bug mention
@ 2009-07-18 00:19 Bruce Momjian <bruce@momjian.us>
0 siblings, 0 replies; 6+ messages in thread
From: Bruce Momjian @ 2009-07-18 00:19 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: Jamie Fox <jfox@directcommerce.com>; pgsql-hackers
To more clearly identify that pg_migrator now has known bugs, I have
released pg_migrator 8.4.1 alpha1, and mentioned in the README that
there are known bugs related to migrating sequences and large objects.
I have removed the 8.4 source file from pgfoundry.
---------------------------------------------------------------------------
Alvaro Herrera wrote:
> Jamie Fox wrote:
>
> > Hi -
> > REINDEX INDEX pg_largeobject_loid_pn_index;
> >
> > This seems to have fixed the problem, lo_open of lob data is working again -
> > now to see how vacuumlo likes it.
>
> So did it work?
>
> --
> Alvaro Herrera http://www.CommandPrompt.com/
> The PostgreSQL Company - Command Prompt, Inc.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v4 12/15] hio: Use ExtendBufferedRelBy()
@ 2022-10-26 21:14 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 6+ messages in thread
From: Andres Freund @ 2022-10-26 21:14 UTC (permalink / raw)
---
src/backend/access/heap/hio.c | 287 +++++++++++++++++-----------------
1 file changed, 147 insertions(+), 140 deletions(-)
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index 65886839e70..48cfcff975f 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -185,90 +185,6 @@ GetVisibilityMapPins(Relation relation, Buffer buffer1, Buffer buffer2,
}
}
-/*
- * Extend a relation by multiple blocks to avoid future contention on the
- * relation extension lock. Our goal is to pre-extend the relation by an
- * amount which ramps up as the degree of contention ramps up, but limiting
- * the result to some sane overall value.
- */
-static void
-RelationAddExtraBlocks(Relation relation, BulkInsertState bistate)
-{
- BlockNumber blockNum,
- firstBlock = InvalidBlockNumber;
- int extraBlocks;
- int lockWaiters;
-
- /* Use the length of the lock wait queue to judge how much to extend. */
- lockWaiters = RelationExtensionLockWaiterCount(relation);
- if (lockWaiters <= 0)
- return;
-
- /*
- * It might seem like multiplying the number of lock waiters by as much as
- * 20 is too aggressive, but benchmarking revealed that smaller numbers
- * were insufficient. 512 is just an arbitrary cap to prevent
- * pathological results.
- */
- extraBlocks = Min(512, lockWaiters * 20);
-
- do
- {
- Buffer buffer;
- Page page;
- Size freespace;
-
- /*
- * Extend by one page. This should generally match the main-line
- * extension code in RelationGetBufferForTuple, except that we hold
- * the relation extension lock throughout, and we don't immediately
- * initialize the page (see below).
- */
- buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
- page = BufferGetPage(buffer);
-
- if (!PageIsNew(page))
- elog(ERROR, "page %u of relation \"%s\" should be empty but is not",
- BufferGetBlockNumber(buffer),
- RelationGetRelationName(relation));
-
- /*
- * Add the page to the FSM without initializing. If we were to
- * initialize here, the page would potentially get flushed out to disk
- * before we add any useful content. There's no guarantee that that'd
- * happen before a potential crash, so we need to deal with
- * uninitialized pages anyway, thus avoid the potential for
- * unnecessary writes.
- */
-
- /* we'll need this info below */
- blockNum = BufferGetBlockNumber(buffer);
- freespace = BufferGetPageSize(buffer) - SizeOfPageHeaderData;
-
- UnlockReleaseBuffer(buffer);
-
- /* Remember first block number thus added. */
- if (firstBlock == InvalidBlockNumber)
- firstBlock = blockNum;
-
- /*
- * Immediately update the bottom level of the FSM. This has a good
- * chance of making this page visible to other concurrently inserting
- * backends, and we want that to happen without delay.
- */
- RecordPageWithFreeSpace(relation, blockNum, freespace);
- }
- while (--extraBlocks > 0);
-
- /*
- * Updating the upper levels of the free space map is too expensive to do
- * for every block, but it's worth doing once at the end to make sure that
- * subsequent insertion activity sees all of those nifty free pages we
- * just inserted.
- */
- FreeSpaceMapVacuumRange(relation, firstBlock, blockNum + 1);
-}
-
/*
* RelationGetBufferForTuple
*
@@ -354,6 +270,9 @@ RelationGetBufferForTuple(Relation relation, Size len,
len = MAXALIGN(len); /* be conservative */
+ if (num_pages <= 0)
+ num_pages = 1;
+
/* Bulk insert is not supported for updates, only inserts. */
Assert(otherBuffer == InvalidBuffer || !bistate);
@@ -558,18 +477,46 @@ loop:
ReleaseBuffer(buffer);
}
- /* Without FSM, always fall out of the loop and extend */
- if (!use_fsm)
- break;
+ if (bistate
+ && bistate->next_free != InvalidBlockNumber
+ && bistate->next_free <= bistate->last_free)
+ {
+ /*
+ * We bulk extended the relation before, and there are still some
+ * unused pages from that extension, so we don't need to look in
+ * the FSM for a new page. But do record the free space from the
+ * last page, somebody might insert narrower tuples later.
+ */
+ if (use_fsm)
+ RecordPageWithFreeSpace(relation, targetBlock, pageFreeSpace);
- /*
- * Update FSM as to condition of this page, and ask for another page
- * to try.
- */
- targetBlock = RecordAndGetPageWithFreeSpace(relation,
- targetBlock,
- pageFreeSpace,
- targetFreeSpace);
+ Assert(bistate->last_free != InvalidBlockNumber &&
+ bistate->next_free <= bistate->last_free);
+ targetBlock = bistate->next_free;
+ if (bistate->next_free >= bistate->last_free)
+ {
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
+ }
+ else
+ bistate->next_free++;
+ }
+ else if (!use_fsm)
+ {
+ /* Without FSM, always fall out of the loop and extend */
+ break;
+ }
+ else
+ {
+ /*
+ * Update FSM as to condition of this page, and ask for another
+ * page to try.
+ */
+ targetBlock = RecordAndGetPageWithFreeSpace(relation,
+ targetBlock,
+ pageFreeSpace,
+ targetFreeSpace);
+ }
}
/*
@@ -582,60 +529,120 @@ loop:
*/
needLock = !RELATION_IS_LOCAL(relation);
- /*
- * If we need the lock but are not able to acquire it immediately, we'll
- * consider extending the relation by multiple blocks at a time to manage
- * contention on the relation extension lock. However, this only makes
- * sense if we're using the FSM; otherwise, there's no point.
- */
- if (needLock)
{
- if (!use_fsm)
- LockRelationForExtension(relation, ExclusiveLock);
- else if (!ConditionalLockRelationForExtension(relation, ExclusiveLock))
+#define MAX_BUFFERS 64
+ Buffer victim_buffers[MAX_BUFFERS];
+ BlockNumber firstBlock = InvalidBlockNumber;
+ BlockNumber firstBlockFSM = InvalidBlockNumber;
+ BlockNumber curBlock;
+ uint32 extend_by_pages;
+ uint32 no_fsm_pages;
+ uint32 waitcount;
+
+ extend_by_pages = num_pages;
+
+ /*
+ * Multiply the number of pages to extend by the number of waiters. Do
+ * this even if we're not using the FSM, as it does relieve
+ * contention. Pages will be found via bistate->next_free.
+ */
+ if (needLock)
+ waitcount = RelationExtensionLockWaiterCount(relation);
+ else
+ waitcount = 0;
+ extend_by_pages += extend_by_pages * waitcount;
+
+ /*
+ * can't extend by more than MAX_BUFFERS, we need to pin them all
+ * concurrently. FIXME: Need an NBuffers / MaxBackends type limit
+ * here.
+ */
+ extend_by_pages = Min(extend_by_pages, MAX_BUFFERS);
+
+ /*
+ * How many of the extended pages not to enter into the FSM.
+ *
+ * Only enter pages that we don't need ourselves into the FSM.
+ * Otherwise every other backend will immediately try to use the pages
+ * this backend neds itself, causing unnecessary contention.
+ *
+ * Bulk extended pages are remembered in bistate->next_free_buffer. So
+ * without a bistate we can't directly make use of them.
+ *
+ * Never enter the page returned into the FSM, we'll immediately use
+ * it.
+ */
+ if (num_pages > 1 && bistate == NULL)
+ no_fsm_pages = 1;
+ else
+ no_fsm_pages = num_pages;
+
+ if (bistate && bistate->current_buf != InvalidBuffer)
{
- /* Couldn't get the lock immediately; wait for it. */
- LockRelationForExtension(relation, ExclusiveLock);
+ ReleaseBuffer(bistate->current_buf);
+ bistate->current_buf = InvalidBuffer;
+ }
- /*
- * Check if some other backend has extended a block for us while
- * we were waiting on the lock.
- */
- targetBlock = GetPageWithFreeSpace(relation, targetFreeSpace);
+ firstBlock = ExtendBufferedRelBy(EB_REL(relation), MAIN_FORKNUM,
+ bistate ? bistate->strategy : NULL,
+ EB_LOCK_FIRST,
+ extend_by_pages,
+ victim_buffers,
+ &extend_by_pages);
- /*
- * If some other waiter has already extended the relation, we
- * don't need to do so; just use the existing freespace.
- */
- if (targetBlock != InvalidBlockNumber)
+ /*
+ * Relation is now extended. Make all but the first buffer available
+ * to other backends.
+ *
+ * XXX: We don't necessarily need to release pin / update FSM while
+ * holding the extension lock. But there are some advantages.
+ */
+ curBlock = firstBlock;
+ for (uint32 i = 0; i < extend_by_pages; i++, curBlock++)
+ {
+ Assert(curBlock == BufferGetBlockNumber(victim_buffers[i]));
+ Assert(BlockNumberIsValid(curBlock));
+
+ /* don't release the pin on the page returned by this function */
+ if (i > 0)
+ ReleaseBuffer(victim_buffers[i]);
+
+ if (i >= no_fsm_pages && use_fsm)
{
- UnlockRelationForExtension(relation, ExclusiveLock);
- goto loop;
- }
+ if (firstBlockFSM == InvalidBlockNumber)
+ firstBlockFSM = curBlock;
- /* Time to bulk-extend. */
- RelationAddExtraBlocks(relation, bistate);
+ RecordPageWithFreeSpace(relation,
+ curBlock,
+ BufferGetPageSize(victim_buffers[i]) - SizeOfPageHeaderData);
+ }
+ }
+
+ if (use_fsm && firstBlockFSM != InvalidBlockNumber)
+ FreeSpaceMapVacuumRange(relation, firstBlockFSM, firstBlock + num_pages);
+
+ if (bistate)
+ {
+ if (extend_by_pages > 1)
+ {
+ bistate->next_free = firstBlock + 1;
+ bistate->last_free = firstBlock + extend_by_pages - 1;
+ }
+ else
+ {
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
+ }
+ }
+
+ buffer = victim_buffers[0];
+ if (bistate)
+ {
+ IncrBufferRefCount(buffer);
+ bistate->current_buf = buffer;
}
}
- /*
- * In addition to whatever extension we performed above, we always add at
- * least one block to satisfy our own request.
- *
- * XXX This does an lseek - rather expensive - but at the moment it is the
- * only way to accurately determine how many blocks are in a relation. Is
- * it worth keeping an accurate file length in shared memory someplace,
- * rather than relying on the kernel to do it for us?
- */
- buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
-
- /*
- * Release the file-extension lock; it's now OK for someone else to extend
- * the relation some more.
- */
- if (needLock)
- UnlockRelationForExtension(relation, ExclusiveLock);
-
/*
* We need to initialize the empty new page. Double-check that it really
* is empty (this should never happen, but if it does we don't want to
--
2.38.0
--njltjzibte523gwd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v4-0013-bufmgr-debug-Add-PrintBuffer-Desc.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v5 11/14] hio: Use ExtendBufferedRelBy()
@ 2022-10-26 21:14 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 6+ messages in thread
From: Andres Freund @ 2022-10-26 21:14 UTC (permalink / raw)
---
src/backend/access/heap/hio.c | 332 ++++++++++++++++++++--------------
1 file changed, 194 insertions(+), 138 deletions(-)
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index 65886839e70..40f53d7177e 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -186,89 +186,176 @@ GetVisibilityMapPins(Relation relation, Buffer buffer1, Buffer buffer2,
}
/*
- * Extend a relation by multiple blocks to avoid future contention on the
- * relation extension lock. Our goal is to pre-extend the relation by an
- * amount which ramps up as the degree of contention ramps up, but limiting
- * the result to some sane overall value.
+ * Extend the relation. By multiple pages, if beneficial.
+ *
+ * If the caller needs multiple pages (num_pages > 1), we always try to extend
+ * by at least that much.
+ *
+ * If there is contention on the extension lock, we don't just extend "for
+ * ourselves", but we try to help others. We can do so by adding empty pages
+ * into the FSM. Typically there is no contention when we can't use the FSM.
+ *
+ * We do have to limit the number of pages to extend by to some value, as the
+ * buffers for all the extended pages need to, temporarily, be pinned. For now
+ * we define MAX_BUFFERS_TO_EXTEND_BY to be 64 buffers, it's hard to see
+ * benefits with higher numbers. This partially is because copyfrom.c's
+ * MAX_BUFFERED_TUPLES / MAX_BUFFERED_BYTES prevents larger multi_inserts.
*/
-static void
-RelationAddExtraBlocks(Relation relation, BulkInsertState bistate)
+static Buffer
+RelationAddBlocks(Relation relation, BulkInsertState bistate,
+ int num_pages, bool use_fsm)
{
- BlockNumber blockNum,
- firstBlock = InvalidBlockNumber;
- int extraBlocks;
- int lockWaiters;
-
- /* Use the length of the lock wait queue to judge how much to extend. */
- lockWaiters = RelationExtensionLockWaiterCount(relation);
- if (lockWaiters <= 0)
- return;
+#define MAX_BUFFERS_TO_EXTEND_BY 64
+ Buffer victim_buffers[MAX_BUFFERS_TO_EXTEND_BY];
+ BlockNumber firstBlock = InvalidBlockNumber;
+ BlockNumber firstBlockFSM = InvalidBlockNumber;
+ uint32 extend_by_pages;
+ uint32 not_in_fsm_pages;
+ BlockNumber curBlock;
+ uint32 waitcount;
+ Buffer buffer;
/*
- * It might seem like multiplying the number of lock waiters by as much as
- * 20 is too aggressive, but benchmarking revealed that smaller numbers
- * were insufficient. 512 is just an arbitrary cap to prevent
- * pathological results.
+ * Determine by how many pages to try to extend by.
*/
- extraBlocks = Min(512, lockWaiters * 20);
-
- do
+ if (bistate == NULL && !use_fsm)
{
- Buffer buffer;
- Page page;
- Size freespace;
-
/*
- * Extend by one page. This should generally match the main-line
- * extension code in RelationGetBufferForTuple, except that we hold
- * the relation extension lock throughout, and we don't immediately
- * initialize the page (see below).
+ * If we have neither bistate, nor can use the FSM, we can't bulk
+ * extend - there'd be no way to find the additional pages.
*/
- buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
- page = BufferGetPage(buffer);
-
- if (!PageIsNew(page))
- elog(ERROR, "page %u of relation \"%s\" should be empty but is not",
- BufferGetBlockNumber(buffer),
- RelationGetRelationName(relation));
-
- /*
- * Add the page to the FSM without initializing. If we were to
- * initialize here, the page would potentially get flushed out to disk
- * before we add any useful content. There's no guarantee that that'd
- * happen before a potential crash, so we need to deal with
- * uninitialized pages anyway, thus avoid the potential for
- * unnecessary writes.
- */
-
- /* we'll need this info below */
- blockNum = BufferGetBlockNumber(buffer);
- freespace = BufferGetPageSize(buffer) - SizeOfPageHeaderData;
-
- UnlockReleaseBuffer(buffer);
-
- /* Remember first block number thus added. */
- if (firstBlock == InvalidBlockNumber)
- firstBlock = blockNum;
-
- /*
- * Immediately update the bottom level of the FSM. This has a good
- * chance of making this page visible to other concurrently inserting
- * backends, and we want that to happen without delay.
- */
- RecordPageWithFreeSpace(relation, blockNum, freespace);
+ extend_by_pages = 1;
+ }
+ else
+ {
+ /*
+ * Try to extend at least by the number of pages the caller needs. We
+ * can remember the additional pages (either via FSM or bistate).
+ */
+ extend_by_pages = num_pages;
+
+ if (!RELATION_IS_LOCAL(relation))
+ waitcount = RelationExtensionLockWaiterCount(relation);
+ else
+ waitcount = 0;
+
+ /*
+ * Multiply the number of pages to extend by the number of waiters. Do
+ * this even if we're not using the FSM, as it still relieves
+ * contention, by deferring the next time this backend needs to
+ * extend. In that case the extended pages will be found via
+ * bistate->next_free.
+ */
+ extend_by_pages += extend_by_pages * waitcount;
+
+ /*
+ * Can't extend by more than MAX_BUFFERS, we need to pin them all
+ * concurrently.
+ */
+ extend_by_pages = Min(extend_by_pages, MAX_BUFFERS_TO_EXTEND_BY);
}
- while (--extraBlocks > 0);
/*
- * Updating the upper levels of the free space map is too expensive to do
- * for every block, but it's worth doing once at the end to make sure that
- * subsequent insertion activity sees all of those nifty free pages we
- * just inserted.
+ * How many of the extended pages should be entered into the FSM?
+ *
+ * If we have a bistate, only enter pages that we don't need ourselves
+ * into the FSM. Otherwise every other backend will immediately try to
+ * use the pages this backend neds itself, causing unnecessary
+ * contention. If we don't have a bistate, we can't avoid the FSM.
+ *
+ * Never enter the page returned into the FSM, we'll immediately use it.
*/
- FreeSpaceMapVacuumRange(relation, firstBlock, blockNum + 1);
+ if (num_pages > 1 && bistate == NULL)
+ not_in_fsm_pages = 1;
+ else
+ not_in_fsm_pages = num_pages;
+
+ /* prepare to put another buffer into the bistate */
+ if (bistate && bistate->current_buf != InvalidBuffer)
+ {
+ ReleaseBuffer(bistate->current_buf);
+ bistate->current_buf = InvalidBuffer;
+ }
+
+ /*
+ * Extend the relation. We ask for the first returned page to be locked,
+ * so that we are sure that nobody has inserted into the page
+ * concurrently.
+ *
+ * With the current MAX_BUFFERS_TO_EXTEND_BY there's no danger of
+ * [auto]vacuum trying to truncate later pages as REL_TRUNCATE_MINIMUM is
+ * way larger.
+ */
+ firstBlock = ExtendBufferedRelBy(EB_REL(relation), MAIN_FORKNUM,
+ bistate ? bistate->strategy : NULL,
+ EB_LOCK_FIRST,
+ extend_by_pages,
+ victim_buffers,
+ &extend_by_pages);
+ /* the buffer the function will return */
+ buffer = victim_buffers[0];
+
+ /*
+ * Relation is now extended. Release pins on all buffers, except for the
+ * first (which we'll return). If we decided to put pages into the FSM,
+ * we can do that as part of the same loop.
+ *
+ * FIXME: Figure out how to better deal with doing this operation while
+ * holding a buffer lock. Likely we can just release the buffer lock (to
+ * reacquire it later) after initializing the page - the target page isn't
+ * in the FSM, so it's going to be very rare that it's found.
+ */
+ curBlock = firstBlock;
+ for (uint32 i = 0; i < extend_by_pages; i++, curBlock++)
+ {
+ Assert(curBlock == BufferGetBlockNumber(victim_buffers[i]));
+ Assert(BlockNumberIsValid(curBlock));
+
+ /* don't release the pin on the page returned by this function */
+ if (i > 0)
+ ReleaseBuffer(victim_buffers[i]);
+
+ if (i >= not_in_fsm_pages && use_fsm)
+ {
+ if (firstBlockFSM == InvalidBlockNumber)
+ firstBlockFSM = curBlock;
+
+ RecordPageWithFreeSpace(relation,
+ curBlock,
+ BufferGetPageSize(victim_buffers[i]) - SizeOfPageHeaderData);
+ }
+ }
+
+ if (use_fsm && firstBlockFSM != InvalidBlockNumber)
+ FreeSpaceMapVacuumRange(relation, firstBlockFSM, firstBlock + num_pages);
+
+ if (bistate)
+ {
+ /*
+ * Remember the pages we extended by so we can use them without
+ * looking into the FSM.
+ */
+ if (extend_by_pages > 1)
+ {
+ bistate->next_free = firstBlock + 1;
+ bistate->last_free = firstBlock + extend_by_pages - 1;
+ }
+ else
+ {
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
+ }
+
+ /* maintain bistate->current_buf */
+ IncrBufferRefCount(buffer);
+ bistate->current_buf = buffer;
+ }
+
+ return buffer;
+#undef MAX_BUFFERS_TO_EXTEND_BY
}
+
/*
* RelationGetBufferForTuple
*
@@ -350,10 +437,12 @@ RelationGetBufferForTuple(Relation relation, Size len,
targetFreeSpace = 0;
BlockNumber targetBlock,
otherBlock;
- bool needLock;
len = MAXALIGN(len); /* be conservative */
+ if (num_pages <= 0)
+ num_pages = 1;
+
/* Bulk insert is not supported for updates, only inserts. */
Assert(otherBuffer == InvalidBuffer || !bistate);
@@ -558,83 +647,50 @@ loop:
ReleaseBuffer(buffer);
}
- /* Without FSM, always fall out of the loop and extend */
- if (!use_fsm)
- break;
-
- /*
- * Update FSM as to condition of this page, and ask for another page
- * to try.
- */
- targetBlock = RecordAndGetPageWithFreeSpace(relation,
- targetBlock,
- pageFreeSpace,
- targetFreeSpace);
- }
-
- /*
- * Have to extend the relation.
- *
- * 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.
- */
- needLock = !RELATION_IS_LOCAL(relation);
-
- /*
- * If we need the lock but are not able to acquire it immediately, we'll
- * consider extending the relation by multiple blocks at a time to manage
- * contention on the relation extension lock. However, this only makes
- * sense if we're using the FSM; otherwise, there's no point.
- */
- if (needLock)
- {
- if (!use_fsm)
- LockRelationForExtension(relation, ExclusiveLock);
- else if (!ConditionalLockRelationForExtension(relation, ExclusiveLock))
+ if (bistate
+ && bistate->next_free != InvalidBlockNumber
+ && bistate->next_free <= bistate->last_free)
{
- /* Couldn't get the lock immediately; wait for it. */
- LockRelationForExtension(relation, ExclusiveLock);
-
/*
- * Check if some other backend has extended a block for us while
- * we were waiting on the lock.
+ * We bulk extended the relation before, and there are still some
+ * unused pages from that extension, so we don't need to look in
+ * the FSM for a new page. But do record the free space from the
+ * last page, somebody might insert narrower tuples later.
*/
- targetBlock = GetPageWithFreeSpace(relation, targetFreeSpace);
+ if (use_fsm)
+ RecordPageWithFreeSpace(relation, targetBlock, pageFreeSpace);
- /*
- * If some other waiter has already extended the relation, we
- * don't need to do so; just use the existing freespace.
- */
- if (targetBlock != InvalidBlockNumber)
+ Assert(bistate->last_free != InvalidBlockNumber &&
+ bistate->next_free <= bistate->last_free);
+ targetBlock = bistate->next_free;
+ if (bistate->next_free >= bistate->last_free)
{
- UnlockRelationForExtension(relation, ExclusiveLock);
- goto loop;
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
}
-
- /* Time to bulk-extend. */
- RelationAddExtraBlocks(relation, bistate);
+ else
+ bistate->next_free++;
+ }
+ else if (!use_fsm)
+ {
+ /* Without FSM, always fall out of the loop and extend */
+ break;
+ }
+ else
+ {
+ /*
+ * Update FSM as to condition of this page, and ask for another
+ * page to try.
+ */
+ targetBlock = RecordAndGetPageWithFreeSpace(relation,
+ targetBlock,
+ pageFreeSpace,
+ targetFreeSpace);
}
}
- /*
- * In addition to whatever extension we performed above, we always add at
- * least one block to satisfy our own request.
- *
- * XXX This does an lseek - rather expensive - but at the moment it is the
- * only way to accurately determine how many blocks are in a relation. Is
- * it worth keeping an accurate file length in shared memory someplace,
- * rather than relying on the kernel to do it for us?
- */
- buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
-
- /*
- * Release the file-extension lock; it's now OK for someone else to extend
- * the relation some more.
- */
- if (needLock)
- UnlockRelationForExtension(relation, ExclusiveLock);
+ /* Have to extend the relation */
+ buffer = RelationAddBlocks(relation, bistate, num_pages, use_fsm);
/*
* We need to initialize the empty new page. Double-check that it really
--
2.38.0
--wwosng5vofi3dmcs
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0012-WIP-Don-t-initialize-page-in-vm-fsm-_extend-not-n.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v5 12/15] hio: Use ExtendBufferedRelBy()
@ 2022-10-26 21:14 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 6+ messages in thread
From: Andres Freund @ 2022-10-26 21:14 UTC (permalink / raw)
---
src/backend/access/heap/hio.c | 285 +++++++++++++++++-----------------
1 file changed, 146 insertions(+), 139 deletions(-)
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index 65886839e7..48cfcff975 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -354,6 +270,9 @@ RelationGetBufferForTuple(Relation relation, Size len,
so in RelationGetBufferForTuple() up above where your changes start,
there is this code
/*
* We first try to put the tuple on the same page we last inserted a tuple
* on, as cached in the BulkInsertState or relcache entry. If that
* doesn't work, we ask the Free Space Map to locate a suitable page.
* Since the FSM's info might be out of date, we have to be prepared to
* loop around and retry multiple times. (To insure this isn't an infinite
* loop, we must update the FSM with the correct amount of free space on
* each page that proves not to be suitable.) If the FSM has no record of
* a page with enough free space, we give up and extend the relation.
*
* When use_fsm is false, we either put the tuple onto the existing target
* page or extend the relation.
*/
if (bistate && bistate->current_buf != InvalidBuffer)
{
targetBlock = BufferGetBlockNumber(bistate->current_buf);
}
else
targetBlock = RelationGetTargetBlock(relation);
if (targetBlock == InvalidBlockNumber && use_fsm)
{
/*
* We have no cached target page, so ask the FSM for an initial
* target.
*/
targetBlock = GetPageWithFreeSpace(relation, targetFreeSpace);
}
And, I was thinking how, ReadBufferBI() only has one caller now
(RelationGetBufferForTuple()) and, this caller basically already has
checked for the case in the inside of ReadBufferBI() (the code I pasted
above)
/* If we have the desired block already pinned, re-pin and return it */
if (bistate->current_buf != InvalidBuffer)
{
if (BufferGetBlockNumber(bistate->current_buf) == targetBlock)
{
/*
* Currently the LOCK variants are only used for extending
* relation, which should never reach this branch.
*/
Assert(mode != RBM_ZERO_AND_LOCK &&
mode != RBM_ZERO_AND_CLEANUP_LOCK);
IncrBufferRefCount(bistate->current_buf);
return bistate->current_buf;
}
/* ... else drop the old buffer */
So, I was thinking maybe there is some way to inline the logic for
ReadBufferBI(), because I think it would feel more streamlined to me.
@@ -558,18 +477,46 @@ loop:
ReleaseBuffer(buffer);
}
Oh, and I forget which commit introduced BulkInsertState->next_free and
last_free, but I remember thinking that it didn't seem to fit with the
other parts of that commit.
- /* Without FSM, always fall out of the loop and extend */
- if (!use_fsm)
- break;
+ if (bistate
+ && bistate->next_free != InvalidBlockNumber
+ && bistate->next_free <= bistate->last_free)
+ {
+ /*
+ * We bulk extended the relation before, and there are still some
+ * unused pages from that extension, so we don't need to look in
+ * the FSM for a new page. But do record the free space from the
+ * last page, somebody might insert narrower tuples later.
+ */
Why couldn't we have found out that we bulk-extended before and get the
block from there up above the while loop?
+ if (use_fsm)
+ RecordPageWithFreeSpace(relation, targetBlock, pageFreeSpace);
- /*
- * Update FSM as to condition of this page, and ask for another page
- * to try.
- */
- targetBlock = RecordAndGetPageWithFreeSpace(relation,
- targetBlock,
- pageFreeSpace,
- targetFreeSpace);
+ Assert(bistate->last_free != InvalidBlockNumber &&
You don't need the below half of the assert.
+ bistate->next_free <= bistate->last_free);
+ targetBlock = bistate->next_free;
+ if (bistate->next_free >= bistate->last_free)
they can only be equal at this point
+ {
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
+ }
+ else
+ bistate->next_free++;
+ }
+ else if (!use_fsm)
+ {
+ /* Without FSM, always fall out of the loop and extend */
+ break;
+ }
It would be nice to have a comment explaining why this is in its own
else if instead of breaking earlier (i.e. !use_fsm is still a valid case
in the if branch above it)
+ else
+ {
+ /*
+ * Update FSM as to condition of this page, and ask for another
+ * page to try.
+ */
+ targetBlock = RecordAndGetPageWithFreeSpace(relation,
+ targetBlock,
+ pageFreeSpace,
+ targetFreeSpace);
+ }
we can get rid of needLock and waitcount variables like this
+#define MAX_BUFFERS 64
+ Buffer victim_buffers[MAX_BUFFERS];
+ BlockNumber firstBlock = InvalidBlockNumber;
+ BlockNumber firstBlockFSM = InvalidBlockNumber;
+ BlockNumber curBlock;
+ uint32 extend_by_pages;
+ uint32 no_fsm_pages;
+ uint32 waitcount;
+
+ extend_by_pages = num_pages;
+
+ /*
+ * Multiply the number of pages to extend by the number of waiters. Do
+ * this even if we're not using the FSM, as it does relieve
+ * contention. Pages will be found via bistate->next_free.
+ */
+ if (needLock)
+ waitcount = RelationExtensionLockWaiterCount(relation);
+ else
+ waitcount = 0;
+ extend_by_pages += extend_by_pages * waitcount;
if (!RELATION_IS_LOCAL(relation))
extend_by_pages += extend_by_pages *
RelationExtensionLockWaiterCount(relation);
+
+ /*
+ * can't extend by more than MAX_BUFFERS, we need to pin them all
+ * concurrently. FIXME: Need an NBuffers / MaxBackends type limit
+ * here.
+ */
+ extend_by_pages = Min(extend_by_pages, MAX_BUFFERS);
+
+ /*
+ * How many of the extended pages not to enter into the FSM.
+ *
+ * Only enter pages that we don't need ourselves into the FSM.
+ * Otherwise every other backend will immediately try to use the pages
+ * this backend neds itself, causing unnecessary contention.
+ *
+ * Bulk extended pages are remembered in bistate->next_free_buffer. So
+ * without a bistate we can't directly make use of them.
+ *
+ * Never enter the page returned into the FSM, we'll immediately use
+ * it.
+ */
+ if (num_pages > 1 && bistate == NULL)
+ no_fsm_pages = 1;
+ else
+ no_fsm_pages = num_pages;
this is more clearly this:
no_fsm_pages = bistate == NULL ? 1 : num_pages;
- /*
- * Release the file-extension lock; it's now OK for someone else to extend
- * the relation some more.
- */
- if (needLock)
- UnlockRelationForExtension(relation, ExclusiveLock);
+ if (bistate)
+ {
+ if (extend_by_pages > 1)
+ {
+ bistate->next_free = firstBlock + 1;
+ bistate->last_free = firstBlock + extend_by_pages - 1;
+ }
+ else
+ {
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
+ }
+ }
+
+ buffer = victim_buffers[0];
If we move buffer = up, we can have only one if (bistate)
+ if (bistate)
+ {
+ IncrBufferRefCount(buffer);
+ bistate->current_buf = buffer;
+ }
+ }
like this:
buffer = victim_buffers[0];
if (bistate)
{
if (extend_by_pages > 1)
{
bistate->next_free = firstBlock + 1;
bistate->last_free = firstBlock + extend_by_pages - 1;
}
else
{
bistate->next_free = InvalidBlockNumber;
bistate->last_free = InvalidBlockNumber;
}
IncrBufferRefCount(buffer);
bistate->current_buf = buffer;
}
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v7 12/14] hio: Use ExtendBufferedRelBy()
@ 2023-03-29 01:39 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 6+ messages in thread
From: Andres Freund @ 2023-03-29 01:39 UTC (permalink / raw)
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de
---
src/backend/access/heap/hio.c | 372 ++++++++++++++++++++--------------
1 file changed, 218 insertions(+), 154 deletions(-)
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index 561d7329058..574c56cfb5f 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -212,87 +212,197 @@ GetVisibilityMapPins(Relation relation, Buffer buffer1, Buffer buffer2,
}
/*
- * Extend a relation by multiple blocks to avoid future contention on the
- * relation extension lock. Our goal is to pre-extend the relation by an
- * amount which ramps up as the degree of contention ramps up, but limiting
- * the result to some sane overall value.
+ * Extend the relation. By multiple pages, if beneficial.
+ *
+ * If the caller needs multiple pages (num_pages > 1), we always try to extend
+ * by at least that much.
+ *
+ * If there is contention on the extension lock, we don't just extend "for
+ * ourselves", but we try to help others. We can do so by adding empty pages
+ * into the FSM. Typically there is no contention when we can't use the FSM.
+ *
+ * We do have to limit the number of pages to extend by to some value, as the
+ * buffers for all the extended pages need to, temporarily, be pinned. For now
+ * we define MAX_BUFFERS_TO_EXTEND_BY to be 64 buffers, it's hard to see
+ * benefits with higher numbers. This partially is because copyfrom.c's
+ * MAX_BUFFERED_TUPLES / MAX_BUFFERED_BYTES prevents larger multi_inserts.
*/
-static void
-RelationAddExtraBlocks(Relation relation, BulkInsertState bistate)
+static Buffer
+RelationAddBlocks(Relation relation, BulkInsertState bistate,
+ int num_pages, bool use_fsm, bool *did_unlock)
{
- BlockNumber blockNum,
- firstBlock = InvalidBlockNumber;
- int extraBlocks;
- int lockWaiters;
-
- /* Use the length of the lock wait queue to judge how much to extend. */
- lockWaiters = RelationExtensionLockWaiterCount(relation);
- if (lockWaiters <= 0)
- return;
+#define MAX_BUFFERS_TO_EXTEND_BY 64
+ Buffer victim_buffers[MAX_BUFFERS_TO_EXTEND_BY];
+ BlockNumber first_block = InvalidBlockNumber;
+ BlockNumber last_block = InvalidBlockNumber;
+ uint32 extend_by_pages;
+ uint32 not_in_fsm_pages;
+ Buffer buffer;
+ Page page;
/*
- * It might seem like multiplying the number of lock waiters by as much as
- * 20 is too aggressive, but benchmarking revealed that smaller numbers
- * were insufficient. 512 is just an arbitrary cap to prevent
- * pathological results.
+ * Determine by how many pages to try to extend by.
*/
- extraBlocks = Min(512, lockWaiters * 20);
-
- do
+ if (bistate == NULL && !use_fsm)
{
- Buffer buffer;
- Page page;
- Size freespace;
-
/*
- * Extend by one page. This should generally match the main-line
- * extension code in RelationGetBufferForTuple, except that we hold
- * the relation extension lock throughout, and we don't immediately
- * initialize the page (see below).
+ * If we have neither bistate, nor can use the FSM, we can't bulk
+ * extend - there'd be no way to find the additional pages.
*/
- buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
- page = BufferGetPage(buffer);
-
- if (!PageIsNew(page))
- elog(ERROR, "page %u of relation \"%s\" should be empty but is not",
- BufferGetBlockNumber(buffer),
- RelationGetRelationName(relation));
-
- /*
- * Add the page to the FSM without initializing. If we were to
- * initialize here, the page would potentially get flushed out to disk
- * before we add any useful content. There's no guarantee that that'd
- * happen before a potential crash, so we need to deal with
- * uninitialized pages anyway, thus avoid the potential for
- * unnecessary writes.
- */
-
- /* we'll need this info below */
- blockNum = BufferGetBlockNumber(buffer);
- freespace = BufferGetPageSize(buffer) - SizeOfPageHeaderData;
-
- UnlockReleaseBuffer(buffer);
-
- /* Remember first block number thus added. */
- if (firstBlock == InvalidBlockNumber)
- firstBlock = blockNum;
-
- /*
- * Immediately update the bottom level of the FSM. This has a good
- * chance of making this page visible to other concurrently inserting
- * backends, and we want that to happen without delay.
- */
- RecordPageWithFreeSpace(relation, blockNum, freespace);
+ extend_by_pages = 1;
+ }
+ else
+ {
+ uint32 waitcount;
+
+ /*
+ * Try to extend at least by the number of pages the caller needs. We
+ * can remember the additional pages (either via FSM or bistate).
+ */
+ extend_by_pages = num_pages;
+
+ if (!RELATION_IS_LOCAL(relation))
+ waitcount = RelationExtensionLockWaiterCount(relation);
+ else
+ waitcount = 0;
+
+ /*
+ * Multiply the number of pages to extend by the number of waiters. Do
+ * this even if we're not using the FSM, as it still relieves
+ * contention, by deferring the next time this backend needs to
+ * extend. In that case the extended pages will be found via
+ * bistate->next_free.
+ */
+ extend_by_pages += extend_by_pages * waitcount;
+
+ /*
+ * Can't extend by more than MAX_BUFFERS_TO_EXTEND_BY, we need to pin
+ * them all concurrently.
+ */
+ extend_by_pages = Min(extend_by_pages, MAX_BUFFERS_TO_EXTEND_BY);
}
- while (--extraBlocks > 0);
/*
- * Updating the upper levels of the free space map is too expensive to do
- * for every block, but it's worth doing once at the end to make sure that
- * subsequent insertion activity sees all of those nifty free pages we
- * just inserted.
+ * How many of the extended pages should be entered into the FSM?
+ *
+ * If we have a bistate, only enter pages that we don't need ourselves
+ * into the FSM. Otherwise every other backend will immediately try to
+ * use the pages this backend needs for itself, causing unnecessary
+ * contention. If we don't have a bistate, we can't avoid the FSM.
+ *
+ * Never enter the page returned into the FSM, we'll immediately use it.
*/
- FreeSpaceMapVacuumRange(relation, firstBlock, blockNum + 1);
+ if (num_pages > 1 && bistate == NULL)
+ not_in_fsm_pages = 1;
+ else
+ not_in_fsm_pages = num_pages;
+
+ /* prepare to put another buffer into the bistate */
+ if (bistate && bistate->current_buf != InvalidBuffer)
+ {
+ ReleaseBuffer(bistate->current_buf);
+ bistate->current_buf = InvalidBuffer;
+ }
+
+ /*
+ * Extend the relation. We ask for the first returned page to be locked,
+ * so that we are sure that nobody has inserted into the page
+ * concurrently.
+ *
+ * With the current MAX_BUFFERS_TO_EXTEND_BY there's no danger of
+ * [auto]vacuum trying to truncate later pages as REL_TRUNCATE_MINIMUM is
+ * way larger.
+ */
+ first_block = ExtendBufferedRelBy(EB_REL(relation), MAIN_FORKNUM,
+ bistate ? bistate->strategy : NULL,
+ EB_LOCK_FIRST,
+ extend_by_pages,
+ victim_buffers,
+ &extend_by_pages);
+ buffer = victim_buffers[0]; /* the buffer the function will return */
+ last_block = first_block + (extend_by_pages - 1);
+ Assert(first_block == BufferGetBlockNumber(buffer));
+
+ /*
+ * Relation is now extended. Initialize the page. We do this here, before
+ * potentially releasing the lock on the page, because it allows us to
+ * double check that the page contents are empty (this should never
+ * happen, but if it does we don't want to risk wiping out valid data).
+ */
+ page = BufferGetPage(buffer);
+ if (!PageIsNew(page))
+ elog(ERROR, "page %u of relation \"%s\" should be empty but is not",
+ first_block,
+ RelationGetRelationName(relation));
+
+ PageInit(page, BufferGetPageSize(buffer), 0);
+ MarkBufferDirty(buffer);
+
+ /*
+ * If we decided to put pages into the FSM, release the buffer lock (but
+ * not pin), we don't want to do IO while holding a buffer lock. This will
+ * necessitate a bit more extensive checking in our caller.
+ */
+ if (use_fsm && not_in_fsm_pages < extend_by_pages)
+ {
+ LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+ *did_unlock = true;
+ }
+
+ /*
+ * Relation is now extended. Release pins on all buffers, except for the
+ * first (which we'll return). If we decided to put pages into the FSM,
+ * we can do that as part of the same loop.
+ */
+ for (uint32 i = 1; i < extend_by_pages; i++)
+ {
+ BlockNumber curBlock = first_block + i;
+
+ Assert(curBlock == BufferGetBlockNumber(victim_buffers[i]));
+ Assert(BlockNumberIsValid(curBlock));
+
+ ReleaseBuffer(victim_buffers[i]);
+
+ if (use_fsm && i >= not_in_fsm_pages)
+ {
+ Size freespace = BufferGetPageSize(victim_buffers[i]) -
+ SizeOfPageHeaderData;
+
+ RecordPageWithFreeSpace(relation, curBlock, freespace);
+ }
+ }
+
+ if (use_fsm && not_in_fsm_pages < extend_by_pages)
+ {
+ BlockNumber first_fsm_block = first_block + not_in_fsm_pages;
+
+ FreeSpaceMapVacuumRange(relation, first_fsm_block, last_block);
+ }
+
+ if (bistate)
+ {
+ /*
+ * Remember the additionaly pages we extended by, so we later can use
+ * them without looking into the FSM.
+ */
+ if (extend_by_pages > 1)
+ {
+ bistate->next_free = first_block + 1;
+ bistate->last_free = last_block;
+ }
+ else
+ {
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
+ }
+
+ /* maintain bistate->current_buf */
+ IncrBufferRefCount(buffer);
+ bistate->current_buf = buffer;
+ }
+
+ return buffer;
+#undef MAX_BUFFERS_TO_EXTEND_BY
}
/*
@@ -376,12 +486,14 @@ RelationGetBufferForTuple(Relation relation, Size len,
targetFreeSpace = 0;
BlockNumber targetBlock,
otherBlock;
- bool needLock;
bool unlockedTargetBuffer;
bool recheckVmPins;
len = MAXALIGN(len); /* be conservative */
+ if (num_pages <= 0)
+ num_pages = 1;
+
/* Bulk insert is not supported for updates, only inserts. */
Assert(otherBuffer == InvalidBuffer || !bistate);
@@ -581,102 +693,54 @@ loop:
ReleaseBuffer(buffer);
}
- /* Without FSM, always fall out of the loop and extend */
- if (!use_fsm)
- break;
-
- /*
- * Update FSM as to condition of this page, and ask for another page
- * to try.
- */
- targetBlock = RecordAndGetPageWithFreeSpace(relation,
- targetBlock,
- pageFreeSpace,
- targetFreeSpace);
- }
-
- /*
- * Have to extend the relation.
- *
- * 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.
- */
- needLock = !RELATION_IS_LOCAL(relation);
-
- /*
- * If we need the lock but are not able to acquire it immediately, we'll
- * consider extending the relation by multiple blocks at a time to manage
- * contention on the relation extension lock. However, this only makes
- * sense if we're using the FSM; otherwise, there's no point.
- */
- if (needLock)
- {
- if (!use_fsm)
- LockRelationForExtension(relation, ExclusiveLock);
- else if (!ConditionalLockRelationForExtension(relation, ExclusiveLock))
+ if (bistate && bistate->next_free != InvalidBlockNumber)
{
- /* Couldn't get the lock immediately; wait for it. */
- LockRelationForExtension(relation, ExclusiveLock);
+ Assert(bistate->next_free <= bistate->last_free);
/*
- * Check if some other backend has extended a block for us while
- * we were waiting on the lock.
+ * We bulk extended the relation before, and there are still some
+ * unused pages from that extension, so we don't need to look in
+ * the FSM for a new page. But do record the free space from the
+ * last page, somebody might insert narrower tuples later.
*/
- targetBlock = GetPageWithFreeSpace(relation, targetFreeSpace);
+ if (use_fsm)
+ RecordPageWithFreeSpace(relation, targetBlock, pageFreeSpace);
- /*
- * If some other waiter has already extended the relation, we
- * don't need to do so; just use the existing freespace.
- */
- if (targetBlock != InvalidBlockNumber)
+ targetBlock = bistate->next_free;
+ if (bistate->next_free >= bistate->last_free)
{
- UnlockRelationForExtension(relation, ExclusiveLock);
- goto loop;
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
}
-
- /* Time to bulk-extend. */
- RelationAddExtraBlocks(relation, bistate);
+ else
+ bistate->next_free++;
+ }
+ else if (!use_fsm)
+ {
+ /* Without FSM, always fall out of the loop and extend */
+ break;
+ }
+ else
+ {
+ /*
+ * Update FSM as to condition of this page, and ask for another
+ * page to try.
+ */
+ targetBlock = RecordAndGetPageWithFreeSpace(relation,
+ targetBlock,
+ pageFreeSpace,
+ targetFreeSpace);
}
}
- /*
- * In addition to whatever extension we performed above, we always add at
- * least one block to satisfy our own request.
- *
- * XXX This does an lseek - rather expensive - but at the moment it is the
- * only way to accurately determine how many blocks are in a relation. Is
- * it worth keeping an accurate file length in shared memory someplace,
- * rather than relying on the kernel to do it for us?
- */
- buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
-
- /*
- * Release the file-extension lock; it's now OK for someone else to extend
- * the relation some more.
- */
- if (needLock)
- UnlockRelationForExtension(relation, ExclusiveLock);
-
+ /* Have to extend the relation */
unlockedTargetBuffer = false;
+ buffer = RelationAddBlocks(relation, bistate, num_pages, use_fsm, &unlockedTargetBuffer);
+ recheckVmPins = unlockedTargetBuffer;
+
targetBlock = BufferGetBlockNumber(buffer);
-
- /*
- * We need to initialize the empty new page. Double-check that it really
- * is empty (this should never happen, but if it does we don't want to
- * risk wiping out valid data).
- */
page = BufferGetPage(buffer);
- if (!PageIsNew(page))
- elog(ERROR, "page %u of relation \"%s\" should be empty but is not",
- targetBlock,
- RelationGetRelationName(relation));
-
- PageInit(page, BufferGetPageSize(buffer), 0);
- MarkBufferDirty(buffer);
-
/*
* The page is empty, pin vmbuffer to set all_frozen bit. We don't want to
* do IO while the buffer is locked, so we unlock the page first if IO is
@@ -688,8 +752,9 @@ loop:
if (!visibilitymap_pin_ok(targetBlock, *vmbuffer))
{
+ if (!unlockedTargetBuffer)
+ LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
unlockedTargetBuffer = true;
- LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
visibilitymap_pin(relation, targetBlock, vmbuffer);
}
}
@@ -702,7 +767,6 @@ loop:
* that another backend used space on this page. We check for that below,
* and retry if necessary.
*/
- recheckVmPins = false;
if (unlockedTargetBuffer)
{
/* released lock on target buffer above */
--
2.38.0
--ju7ntqqtbf66a3ug
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v7-0013-Convert-a-few-places-to-ExtendBufferedRelTo.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v6 14/17] hio: Use ExtendBufferedRelBy()
@ 2023-03-29 01:39 Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 6+ messages in thread
From: Andres Freund @ 2023-03-29 01:39 UTC (permalink / raw)
---
src/backend/access/heap/hio.c | 370 ++++++++++++++++++++--------------
1 file changed, 217 insertions(+), 153 deletions(-)
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index cc913a028e0..6d66826d951 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -192,87 +192,197 @@ GetVisibilityMapPins(Relation relation, Buffer buffer1, Buffer buffer2,
}
/*
- * Extend a relation by multiple blocks to avoid future contention on the
- * relation extension lock. Our goal is to pre-extend the relation by an
- * amount which ramps up as the degree of contention ramps up, but limiting
- * the result to some sane overall value.
+ * Extend the relation. By multiple pages, if beneficial.
+ *
+ * If the caller needs multiple pages (num_pages > 1), we always try to extend
+ * by at least that much.
+ *
+ * If there is contention on the extension lock, we don't just extend "for
+ * ourselves", but we try to help others. We can do so by adding empty pages
+ * into the FSM. Typically there is no contention when we can't use the FSM.
+ *
+ * We do have to limit the number of pages to extend by to some value, as the
+ * buffers for all the extended pages need to, temporarily, be pinned. For now
+ * we define MAX_BUFFERS_TO_EXTEND_BY to be 64 buffers, it's hard to see
+ * benefits with higher numbers. This partially is because copyfrom.c's
+ * MAX_BUFFERED_TUPLES / MAX_BUFFERED_BYTES prevents larger multi_inserts.
*/
-static void
-RelationAddExtraBlocks(Relation relation, BulkInsertState bistate)
+static Buffer
+RelationAddBlocks(Relation relation, BulkInsertState bistate,
+ int num_pages, bool use_fsm, bool *did_unlock)
{
- BlockNumber blockNum,
- firstBlock = InvalidBlockNumber;
- int extraBlocks;
- int lockWaiters;
-
- /* Use the length of the lock wait queue to judge how much to extend. */
- lockWaiters = RelationExtensionLockWaiterCount(relation);
- if (lockWaiters <= 0)
- return;
+#define MAX_BUFFERS_TO_EXTEND_BY 64
+ Buffer victim_buffers[MAX_BUFFERS_TO_EXTEND_BY];
+ BlockNumber first_block = InvalidBlockNumber;
+ BlockNumber last_block = InvalidBlockNumber;
+ uint32 extend_by_pages;
+ uint32 not_in_fsm_pages;
+ Buffer buffer;
+ Page page;
/*
- * It might seem like multiplying the number of lock waiters by as much as
- * 20 is too aggressive, but benchmarking revealed that smaller numbers
- * were insufficient. 512 is just an arbitrary cap to prevent
- * pathological results.
+ * Determine by how many pages to try to extend by.
*/
- extraBlocks = Min(512, lockWaiters * 20);
-
- do
+ if (bistate == NULL && !use_fsm)
{
- Buffer buffer;
- Page page;
- Size freespace;
-
/*
- * Extend by one page. This should generally match the main-line
- * extension code in RelationGetBufferForTuple, except that we hold
- * the relation extension lock throughout, and we don't immediately
- * initialize the page (see below).
+ * If we have neither bistate, nor can use the FSM, we can't bulk
+ * extend - there'd be no way to find the additional pages.
*/
- buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
- page = BufferGetPage(buffer);
-
- if (!PageIsNew(page))
- elog(ERROR, "page %u of relation \"%s\" should be empty but is not",
- BufferGetBlockNumber(buffer),
- RelationGetRelationName(relation));
-
- /*
- * Add the page to the FSM without initializing. If we were to
- * initialize here, the page would potentially get flushed out to disk
- * before we add any useful content. There's no guarantee that that'd
- * happen before a potential crash, so we need to deal with
- * uninitialized pages anyway, thus avoid the potential for
- * unnecessary writes.
- */
-
- /* we'll need this info below */
- blockNum = BufferGetBlockNumber(buffer);
- freespace = BufferGetPageSize(buffer) - SizeOfPageHeaderData;
-
- UnlockReleaseBuffer(buffer);
-
- /* Remember first block number thus added. */
- if (firstBlock == InvalidBlockNumber)
- firstBlock = blockNum;
-
- /*
- * Immediately update the bottom level of the FSM. This has a good
- * chance of making this page visible to other concurrently inserting
- * backends, and we want that to happen without delay.
- */
- RecordPageWithFreeSpace(relation, blockNum, freespace);
+ extend_by_pages = 1;
+ }
+ else
+ {
+ uint32 waitcount;
+
+ /*
+ * Try to extend at least by the number of pages the caller needs. We
+ * can remember the additional pages (either via FSM or bistate).
+ */
+ extend_by_pages = num_pages;
+
+ if (!RELATION_IS_LOCAL(relation))
+ waitcount = RelationExtensionLockWaiterCount(relation);
+ else
+ waitcount = 0;
+
+ /*
+ * Multiply the number of pages to extend by the number of waiters. Do
+ * this even if we're not using the FSM, as it still relieves
+ * contention, by deferring the next time this backend needs to
+ * extend. In that case the extended pages will be found via
+ * bistate->next_free.
+ */
+ extend_by_pages += extend_by_pages * waitcount;
+
+ /*
+ * Can't extend by more than MAX_BUFFERS_TO_EXTEND_BY, we need to pin
+ * them all concurrently.
+ */
+ extend_by_pages = Min(extend_by_pages, MAX_BUFFERS_TO_EXTEND_BY);
}
- while (--extraBlocks > 0);
/*
- * Updating the upper levels of the free space map is too expensive to do
- * for every block, but it's worth doing once at the end to make sure that
- * subsequent insertion activity sees all of those nifty free pages we
- * just inserted.
+ * How many of the extended pages should be entered into the FSM?
+ *
+ * If we have a bistate, only enter pages that we don't need ourselves
+ * into the FSM. Otherwise every other backend will immediately try to
+ * use the pages this backend needs for itself, causing unnecessary
+ * contention. If we don't have a bistate, we can't avoid the FSM.
+ *
+ * Never enter the page returned into the FSM, we'll immediately use it.
*/
- FreeSpaceMapVacuumRange(relation, firstBlock, blockNum + 1);
+ if (num_pages > 1 && bistate == NULL)
+ not_in_fsm_pages = 1;
+ else
+ not_in_fsm_pages = num_pages;
+
+ /* prepare to put another buffer into the bistate */
+ if (bistate && bistate->current_buf != InvalidBuffer)
+ {
+ ReleaseBuffer(bistate->current_buf);
+ bistate->current_buf = InvalidBuffer;
+ }
+
+ /*
+ * Extend the relation. We ask for the first returned page to be locked,
+ * so that we are sure that nobody has inserted into the page
+ * concurrently.
+ *
+ * With the current MAX_BUFFERS_TO_EXTEND_BY there's no danger of
+ * [auto]vacuum trying to truncate later pages as REL_TRUNCATE_MINIMUM is
+ * way larger.
+ */
+ first_block = ExtendBufferedRelBy(EB_REL(relation), MAIN_FORKNUM,
+ bistate ? bistate->strategy : NULL,
+ EB_LOCK_FIRST,
+ extend_by_pages,
+ victim_buffers,
+ &extend_by_pages);
+ buffer = victim_buffers[0]; /* the buffer the function will return */
+ last_block = first_block + (extend_by_pages - 1);
+ Assert(first_block == BufferGetBlockNumber(buffer));
+
+ /*
+ * Relation is now extended. Initialize the page. We do this here, before
+ * potentially releasing the lock on the page, because it allows us to
+ * double check that the page contents are empty (this should never
+ * happen, but if it does we don't want to risk wiping out valid data).
+ */
+ page = BufferGetPage(buffer);
+ if (!PageIsNew(page))
+ elog(ERROR, "page %u of relation \"%s\" should be empty but is not",
+ first_block,
+ RelationGetRelationName(relation));
+
+ PageInit(page, BufferGetPageSize(buffer), 0);
+ MarkBufferDirty(buffer);
+
+ /*
+ * If we decided to put pages into the FSM, release the buffer lock (but
+ * not pin), we don't want to do IO while holding a buffer lock. This will
+ * necessitate a bit more extensive checking in our caller.
+ */
+ if (use_fsm && not_in_fsm_pages < extend_by_pages)
+ {
+ LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+ *did_unlock = true;
+ }
+
+ /*
+ * Relation is now extended. Release pins on all buffers, except for the
+ * first (which we'll return). If we decided to put pages into the FSM,
+ * we can do that as part of the same loop.
+ */
+ for (uint32 i = 1; i < extend_by_pages; i++)
+ {
+ BlockNumber curBlock = first_block + i;
+
+ Assert(curBlock == BufferGetBlockNumber(victim_buffers[i]));
+ Assert(BlockNumberIsValid(curBlock));
+
+ ReleaseBuffer(victim_buffers[i]);
+
+ if (use_fsm && i >= not_in_fsm_pages)
+ {
+ Size freespace = BufferGetPageSize(victim_buffers[i]) -
+ SizeOfPageHeaderData;
+
+ RecordPageWithFreeSpace(relation, curBlock, freespace);
+ }
+ }
+
+ if (use_fsm && not_in_fsm_pages < extend_by_pages)
+ {
+ BlockNumber first_fsm_block = first_block + not_in_fsm_pages;
+
+ FreeSpaceMapVacuumRange(relation, first_fsm_block, last_block);
+ }
+
+ if (bistate)
+ {
+ /*
+ * Remember the additionaly pages we extended by, so we later can use
+ * them without looking into the FSM.
+ */
+ if (extend_by_pages > 1)
+ {
+ bistate->next_free = first_block + 1;
+ bistate->last_free = last_block;
+ }
+ else
+ {
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
+ }
+
+ /* maintain bistate->current_buf */
+ IncrBufferRefCount(buffer);
+ bistate->current_buf = buffer;
+ }
+
+ return buffer;
+#undef MAX_BUFFERS_TO_EXTEND_BY
}
/*
@@ -356,11 +466,13 @@ RelationGetBufferForTuple(Relation relation, Size len,
targetFreeSpace = 0;
BlockNumber targetBlock,
otherBlock;
- bool needLock;
bool unlockedTargetBuffer;
len = MAXALIGN(len); /* be conservative */
+ if (num_pages <= 0)
+ num_pages = 1;
+
/* Bulk insert is not supported for updates, only inserts. */
Assert(otherBuffer == InvalidBuffer || !bistate);
@@ -565,102 +677,53 @@ loop:
ReleaseBuffer(buffer);
}
- /* Without FSM, always fall out of the loop and extend */
- if (!use_fsm)
- break;
-
- /*
- * Update FSM as to condition of this page, and ask for another page
- * to try.
- */
- targetBlock = RecordAndGetPageWithFreeSpace(relation,
- targetBlock,
- pageFreeSpace,
- targetFreeSpace);
- }
-
- /*
- * Have to extend the relation.
- *
- * 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.
- */
- needLock = !RELATION_IS_LOCAL(relation);
-
- /*
- * If we need the lock but are not able to acquire it immediately, we'll
- * consider extending the relation by multiple blocks at a time to manage
- * contention on the relation extension lock. However, this only makes
- * sense if we're using the FSM; otherwise, there's no point.
- */
- if (needLock)
- {
- if (!use_fsm)
- LockRelationForExtension(relation, ExclusiveLock);
- else if (!ConditionalLockRelationForExtension(relation, ExclusiveLock))
+ if (bistate && bistate->next_free != InvalidBlockNumber)
{
- /* Couldn't get the lock immediately; wait for it. */
- LockRelationForExtension(relation, ExclusiveLock);
+ Assert(bistate->next_free <= bistate->last_free);
/*
- * Check if some other backend has extended a block for us while
- * we were waiting on the lock.
+ * We bulk extended the relation before, and there are still some
+ * unused pages from that extension, so we don't need to look in
+ * the FSM for a new page. But do record the free space from the
+ * last page, somebody might insert narrower tuples later.
*/
- targetBlock = GetPageWithFreeSpace(relation, targetFreeSpace);
+ if (use_fsm)
+ RecordPageWithFreeSpace(relation, targetBlock, pageFreeSpace);
- /*
- * If some other waiter has already extended the relation, we
- * don't need to do so; just use the existing freespace.
- */
- if (targetBlock != InvalidBlockNumber)
+ targetBlock = bistate->next_free;
+ if (bistate->next_free >= bistate->last_free)
{
- UnlockRelationForExtension(relation, ExclusiveLock);
- goto loop;
+ bistate->next_free = InvalidBlockNumber;
+ bistate->last_free = InvalidBlockNumber;
}
-
- /* Time to bulk-extend. */
- RelationAddExtraBlocks(relation, bistate);
+ else
+ bistate->next_free++;
+ }
+ else if (!use_fsm)
+ {
+ /* Without FSM, always fall out of the loop and extend */
+ break;
+ }
+ else
+ {
+ /*
+ * Update FSM as to condition of this page, and ask for another
+ * page to try.
+ */
+ targetBlock = RecordAndGetPageWithFreeSpace(relation,
+ targetBlock,
+ pageFreeSpace,
+ targetFreeSpace);
}
}
- /*
- * In addition to whatever extension we performed above, we always add at
- * least one block to satisfy our own request.
- *
- * XXX This does an lseek - rather expensive - but at the moment it is the
- * only way to accurately determine how many blocks are in a relation. Is
- * it worth keeping an accurate file length in shared memory someplace,
- * rather than relying on the kernel to do it for us?
- */
- buffer = ReadBufferBI(relation, P_NEW, RBM_ZERO_AND_LOCK, bistate);
-
- /*
- * Release the file-extension lock; it's now OK for someone else to extend
- * the relation some more.
- */
- if (needLock)
- UnlockRelationForExtension(relation, ExclusiveLock);
-
+ /* Have to extend the relation */
unlockedTargetBuffer = false;
+ buffer = RelationAddBlocks(relation, bistate, num_pages, use_fsm, &unlockedTargetBuffer);
+
targetBlock = BufferGetBlockNumber(buffer);
-
- /*
- * We need to initialize the empty new page. Double-check that it really
- * is empty (this should never happen, but if it does we don't want to
- * risk wiping out valid data).
- */
page = BufferGetPage(buffer);
- if (!PageIsNew(page))
- elog(ERROR, "page %u of relation \"%s\" should be empty but is not",
- targetBlock,
- RelationGetRelationName(relation));
-
- PageInit(page, BufferGetPageSize(buffer), 0);
- MarkBufferDirty(buffer);
-
/*
* The page is empty, pin vmbuffer to set all_frozen bit. We don't want to
* do IO while the buffer is locked, so we unlock the page first if IO is
@@ -672,8 +735,9 @@ loop:
if (!visibilitymap_pin_ok(targetBlock, *vmbuffer))
{
+ if (!unlockedTargetBuffer)
+ LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
unlockedTargetBuffer = true;
- LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
visibilitymap_pin(relation, targetBlock, vmbuffer);
}
}
--
2.38.0
--cfn72vqnlbycypta
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0015-WIP-Don-t-initialize-page-in-vm-fsm-_extend-not-n.patch"
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2023-03-29 01:39 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2009-07-18 00:19 pg_migrator 8.4.1 alpha 1 released with bug mention Bruce Momjian <bruce@momjian.us>
2022-10-26 21:14 [PATCH v4 12/15] hio: Use ExtendBufferedRelBy() Andres Freund <andres@anarazel.de>
2022-10-26 21:14 [PATCH v5 11/14] hio: Use ExtendBufferedRelBy() Andres Freund <andres@anarazel.de>
2022-10-26 21:14 [PATCH v5 12/15] hio: Use ExtendBufferedRelBy() Andres Freund <andres@anarazel.de>
2023-03-29 01:39 [PATCH v7 12/14] hio: Use ExtendBufferedRelBy() Andres Freund <andres@anarazel.de>
2023-03-29 01:39 [PATCH v6 14/17] hio: Use ExtendBufferedRelBy() Andres Freund <andres@anarazel.de>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox