I have attached a v3 which includes two commits -- one of which
implements the directmgr API and uses it and the other which adds
functionality to use either directmgr or bufmgr API during index build.
Also registering for march commitfest.
On Thu, Dec 9, 2021 at 2:33 PM Andres Freund <and...@anarazel.de> wrote:
>
> Hi,
>
> On 2021-11-19 15:11:57 -0500, Melanie Plageman wrote:
> > From 2130175c5d794f60c5f15d6eb1b626c81fb7c39b Mon Sep 17 00:00:00 2001
> > From: Melanie Plageman <melanieplage...@gmail.com>
> > Date: Thu, 15 Apr 2021 07:01:01 -0400
> > Subject: [PATCH v2] Index build avoids immed fsync
> >
> > Avoid immediate fsync for just built indexes either by using shared
> > buffers or by leveraging checkpointer's SyncRequest queue. When a
> > checkpoint begins during the index build, if not using shared buffers,
> > the backend will have to do its own fsync.
> > ---
> > src/backend/access/nbtree/nbtree.c | 39 +++---
> > src/backend/access/nbtree/nbtsort.c | 186 +++++++++++++++++++++++-----
> > src/backend/access/transam/xlog.c | 14 +++
> > src/include/access/xlog.h | 1 +
> > 4 files changed, 188 insertions(+), 52 deletions(-)
> >
> > diff --git a/src/backend/access/nbtree/nbtree.c
> > b/src/backend/access/nbtree/nbtree.c
> > index 40ad0956e0..a2e32f18e6 100644
> > --- a/src/backend/access/nbtree/nbtree.c
> > +++ b/src/backend/access/nbtree/nbtree.c
> > @@ -150,30 +150,29 @@ void
> > btbuildempty(Relation index)
> > {
> > Page metapage;
> > + Buffer metabuf;
> >
> > - /* Construct metapage. */
> > - metapage = (Page) palloc(BLCKSZ);
> > - _bt_initmetapage(metapage, P_NONE, 0, _bt_allequalimage(index,
> > false));
> > -
> > + // TODO: test this.
>
> Shouldn't this path have plenty coverage?
Yep. Sorry.
> > /*
> > - * Write the page and log it. It might seem that an immediate sync
> > would
> > - * be sufficient to guarantee that the file exists on disk, but
> > recovery
> > - * itself might remove it while replaying, for example, an
> > - * XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record. Therefore, we need
> > - * this even when wal_level=minimal.
> > + * Construct metapage.
> > + * Because we don't need to lock the relation for extension (since
> > + * noone knows about it yet) and we don't need to initialize the
> > + * new page, as it is done below by _bt_blnewpage(), _bt_getbuf()
> > + * (with P_NEW and BT_WRITE) is overkill.
>
> Isn't the more relevant operation the log_newpage_buffer()?
Returning to this after some time away, many of my comments no longer
make sense to me either. I can't actually tell which diff your question
applies to because this comment was copy-pasted in two different places
in my code. Also, I've removed this comment and added new ones. So,
given all that, is there still something about log_newpage_buffer() I
should be commenting on?
> > However, it might be worth
> > + * either modifying it or adding a new helper function instead of
> > + * calling ReadBufferExtended() directly. We pass mode
> > RBM_ZERO_AND_LOCK
> > + * because we want to hold an exclusive lock on the buffer content
> > */
>
> "modifying it" refers to what?
>
> I don't see a problem using ReadBufferExtended() here. Why would you like to
> replace it with something else?
I would just disregard these comments now.
> > + /*
> > + * Based on the number of tuples, either create a buffered or
> > unbuffered
> > + * write state. if the number of tuples is small, make a buffered
> > write
> > + * if the number of tuples is larger, then we make an unbuffered
> > write state
> > + * and must ensure that we check the redo pointer to know whether or
> > not we
> > + * need to fsync ourselves
> > + */
> >
> > /*
> > * Finish the build by (1) completing the sort of the spool file, (2)
> > * inserting the sorted tuples into btree pages and (3) building the
> > upper
> > * levels. Finally, it may also be necessary to end use of
> > parallelism.
> > */
> > - _bt_leafbuild(buildstate.spool, buildstate.spool2);
> > + if (reltuples > 1000)
>
> I'm ok with some random magic constant here, but it seems worht putting it in
> some constant / #define to make it more obvious.
Done.
> > + _bt_leafbuild(buildstate.spool, buildstate.spool2, false);
> > + else
> > + _bt_leafbuild(buildstate.spool, buildstate.spool2, true);
>
> Why duplicate the function call?
Fixed.
> > /*
> > * allocate workspace for a new, clean btree page, not linked to any
> > siblings.
> > + * If index is not built in shared buffers, buf should be InvalidBuffer
> > */
> > static Page
> > -_bt_blnewpage(uint32 level)
> > +_bt_blnewpage(uint32 level, Buffer buf)
> > {
> > Page page;
> > BTPageOpaque opaque;
> >
> > - page = (Page) palloc(BLCKSZ);
> > + if (buf)
> > + page = BufferGetPage(buf);
> > + else
> > + page = (Page) palloc(BLCKSZ);
> >
> > /* Zero the page and set up standard page header info */
> > _bt_pageinit(page, BLCKSZ);
>
> Ick, that seems pretty ugly API-wise and subsequently too likely to lead to
> pfree()ing a page that's actually in shared buffers. I think it'd make sense
> to separate the allocation from the initialization bits?
Fixed.
> > @@ -635,8 +657,20 @@ _bt_blnewpage(uint32 level)
> > * emit a completed btree page, and release the working storage.
> > */
> > static void
> > -_bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
> > +_bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno, Buffer
> > buf)
> > {
> > + if (wstate->use_shared_buffers)
> > + {
> > + Assert(buf);
> > + START_CRIT_SECTION();
> > + MarkBufferDirty(buf);
> > + if (wstate->btws_use_wal)
> > + log_newpage_buffer(buf, true);
> > + END_CRIT_SECTION();
> > + _bt_relbuf(wstate->index, buf);
> > + return;
> > + }
> > +
> > /* XLOG stuff */
> > if (wstate->btws_use_wal)
> > {
> > @@ -659,7 +693,7 @@ _bt_blwritepage(BTWriteState *wstate, Page page,
> > BlockNumber blkno)
> > smgrextend(RelationGetSmgr(wstate->index), MAIN_FORKNUM,
> > wstate->btws_pages_written++,
> > (char *) wstate->btws_zeropage,
> > - true);
> > + false);
> > }
>
> Is there a good place to document the way we ensure durability for this path?
I added some new comments. Let me know if you think that I am still
missing this documentation.
> > + /*
> > + * Extend the index relation upfront to reserve the metapage
> > + */
> > + if (wstate->use_shared_buffers)
> > + {
> > + /*
> > + * We should not need to LockRelationForExtension() as no one
> > else knows
> > + * about this index yet?
> > + * Extend the index relation by one block for the metapage.
> > _bt_getbuf()
> > + * is not used here as it does _bt_pageinit() which is one
> > later by
>
> *done
>
>
> > + * _bt_initmetapage(). We will fill in the metapage and write
> > it out at
> > + * the end of index build when we have all of the information
> > required
> > + * for the metapage. However, we initially extend the
> > relation for it to
> > + * occupy block 0 because it is much easier when using shared
> > buffers to
> > + * extend the relation with a block number that is always
> > increasing by
> > + * 1.
>
> Not quite following what you're trying to get at here. There generally is no
> way to extend a relation except by increasing block numbers?
I've updated this comment too. It should make more sense now.
> > @@ -1425,7 +1544,10 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool,
> > BTSpool *btspool2)
> > * still not be on disk when the crash occurs.
> > */
> > if (wstate->btws_use_wal)
> > - smgrimmedsync(RelationGetSmgr(wstate->index), MAIN_FORKNUM);
> > + {
> > + if (!wstate->use_shared_buffers &&
> > RedoRecPtrChanged(wstate->redo))
> > + smgrimmedsync(RelationGetSmgr(wstate->index),
> > MAIN_FORKNUM);
> > + }
> > }
> >
> > /*
>
> This needs documentation. The whole comment above isn't accurate anymore
> afaict?
Should be correct now.
> > diff --git a/src/backend/access/transam/xlog.c
> > b/src/backend/access/transam/xlog.c
> > index 1616448368..63fd212787 100644
> > --- a/src/backend/access/transam/xlog.c
> > +++ b/src/backend/access/transam/xlog.c
> > @@ -8704,6 +8704,20 @@ GetRedoRecPtr(void)
> > return RedoRecPtr;
> > }
> >
> > +bool
> > +RedoRecPtrChanged(XLogRecPtr comparator_ptr)
> > +{
> > + XLogRecPtr ptr;
> > +
> > + SpinLockAcquire(&XLogCtl->info_lck);
> > + ptr = XLogCtl->RedoRecPtr;
> > + SpinLockRelease(&XLogCtl->info_lck);
> > +
> > + if (RedoRecPtr < ptr)
> > + RedoRecPtr = ptr;
> > + return RedoRecPtr != comparator_ptr;
> > +}
>
> What's the deal with the < comparison?
I saw that GetRedoRecPtr() does this and thought maybe I should do the
same in this function. I'm not quite sure where I should be getting the
redo pointer.
Maybe I should just call GetRedoRecPtr() and compare it to the one I
saved? I suppose I also thought that maybe someone else in the future
would like to have a function like RedoRecPtrChanged().
- Melanie
From 492d9d088df670a837efb2c107114457663378d8 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Tue, 28 Sep 2021 14:51:11 -0400
Subject: [PATCH v3 1/2] Add unbuffered IO and avoid immed fsync
Replace unbuffered extends and writes
When writing data outside of shared buffers, the backend must do a
series of steps to ensure the data is both durable and recoverable. When
writing or extending a page of data, the backend must log, checksum, and
write out the page before freeing it.
Additionally, though the data is durable once the WAL entries are on
permanent storage, the XLOG Redo pointer cannot be moved past this WAL
until the page data is safely on permanent storage. If a crash were to
occur before the data is fsync'd, the WAL wouldn't be replayed during
recovery, and the data would be lost. Thus, the backend must ensure that
either the Redo pointer has not moved or that the data is fsync'd before
freeing the page.
This is not a problem with pages written in shared buffers because the
checkpointer will block until all buffers that were dirtied before it
began finish before it moves the Redo pointer past their associated WAL
entries.
This commit makes two main changes:
1) It wraps smgrextend() and smgrwrite() in functions from a new API
for writing data outside of shared buffers, directmgr.
2) It saves the XLOG Redo pointer location before doing the write or
extend. It also adds an fsync request for the page to the
checkpointer's pending-ops table. Then, after doing the write or
extend, if the Redo pointer has moved (meaning a checkpoint has
started since it saved it last), then the backend fsync's the page
itself. Otherwise, it lets the checkpointer take care of fsync'ing
the page the next time it processes the pending-ops table.
---
src/backend/access/gist/gistbuild.c | 17 +++----
src/backend/access/hash/hashpage.c | 9 ++--
src/backend/access/heap/heapam_handler.c | 16 ++++---
src/backend/access/heap/rewriteheap.c | 26 ++++-------
src/backend/access/heap/visibilitymap.c | 8 ++--
src/backend/access/nbtree/nbtree.c | 17 ++++---
src/backend/access/nbtree/nbtsort.c | 39 +++++-----------
src/backend/access/spgist/spginsert.c | 25 +++++-----
src/backend/access/transam/xlog.c | 13 ++++++
src/backend/catalog/storage.c | 25 +++-------
src/backend/storage/Makefile | 2 +-
src/backend/storage/direct/Makefile | 17 +++++++
src/backend/storage/direct/directmgr.c | 57 +++++++++++++++++++++++
src/backend/storage/freespace/freespace.c | 10 ++--
src/include/access/xlog.h | 1 +
15 files changed, 175 insertions(+), 107 deletions(-)
create mode 100644 src/backend/storage/direct/Makefile
create mode 100644 src/backend/storage/direct/directmgr.c
diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index 9854116fca..748ca65492 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -43,6 +43,7 @@
#include "miscadmin.h"
#include "optimizer/optimizer.h"
#include "storage/bufmgr.h"
+#include "storage/directmgr.h"
#include "storage/smgr.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -91,6 +92,7 @@ typedef struct
int64 indtuples; /* number of tuples indexed */
+ UnBufferedWriteState ub_wstate;
/*
* Extra data structures used during a buffering build. 'gfbb' contains
* information related to managing the build buffers. 'parentMap' is a
@@ -194,6 +196,7 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
buildstate.heaprel = heap;
buildstate.sortstate = NULL;
buildstate.giststate = initGISTstate(index);
+ buildstate.ub_wstate.smgr_rel = RelationGetSmgr(index);
/*
* Create a temporary memory context that is reset once for each tuple
@@ -403,14 +406,15 @@ gist_indexsortbuild(GISTBuildState *state)
state->pages_allocated = 0;
state->pages_written = 0;
state->ready_num_pages = 0;
+ unbuffered_prep(&state->ub_wstate);
/*
* Write an empty page as a placeholder for the root page. It will be
* replaced with the real root page at the end.
*/
page = palloc0(BLCKSZ);
- smgrextend(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, GIST_ROOT_BLKNO,
- page, true);
+ unbuffered_extend(&state->ub_wstate, MAIN_FORKNUM, GIST_ROOT_BLKNO, page,
+ true);
state->pages_allocated++;
state->pages_written++;
@@ -450,13 +454,12 @@ gist_indexsortbuild(GISTBuildState *state)
/* Write out the root */
PageSetLSN(pagestate->page, GistBuildLSN);
- PageSetChecksumInplace(pagestate->page, GIST_ROOT_BLKNO);
- smgrwrite(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, GIST_ROOT_BLKNO,
- pagestate->page, true);
+ unbuffered_write(&state->ub_wstate, MAIN_FORKNUM, GIST_ROOT_BLKNO, pagestate->page);
if (RelationNeedsWAL(state->indexrel))
log_newpage(&state->indexrel->rd_node, MAIN_FORKNUM, GIST_ROOT_BLKNO,
pagestate->page, true);
+ unbuffered_finish(&state->ub_wstate, MAIN_FORKNUM);
pfree(pagestate->page);
pfree(pagestate);
}
@@ -570,9 +573,7 @@ gist_indexsortbuild_flush_ready_pages(GISTBuildState *state)
elog(ERROR, "unexpected block number to flush GiST sorting build");
PageSetLSN(page, GistBuildLSN);
- PageSetChecksumInplace(page, blkno);
- smgrextend(RelationGetSmgr(state->indexrel), MAIN_FORKNUM, blkno, page,
- true);
+ unbuffered_extend(&state->ub_wstate, MAIN_FORKNUM, blkno, page, false);
state->pages_written++;
}
diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index ee351aea09..722420adf5 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -32,6 +32,7 @@
#include "access/hash_xlog.h"
#include "miscadmin.h"
#include "port/pg_bitutils.h"
+#include "storage/directmgr.h"
#include "storage/lmgr.h"
#include "storage/predicate.h"
#include "storage/smgr.h"
@@ -990,8 +991,10 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
PGAlignedBlock zerobuf;
Page page;
HashPageOpaque ovflopaque;
+ UnBufferedWriteState ub_wstate;
lastblock = firstblock + nblocks - 1;
+ ub_wstate.smgr_rel = RelationGetSmgr(rel);
/*
* Check for overflow in block number calculation; if so, we cannot extend
@@ -1000,6 +1003,8 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
if (lastblock < firstblock || lastblock == InvalidBlockNumber)
return false;
+ unbuffered_prep(&ub_wstate);
+
page = (Page) zerobuf.data;
/*
@@ -1024,9 +1029,7 @@ _hash_alloc_buckets(Relation rel, BlockNumber firstblock, uint32 nblocks)
zerobuf.data,
true);
- PageSetChecksumInplace(page, lastblock);
- smgrextend(RelationGetSmgr(rel), MAIN_FORKNUM, lastblock, zerobuf.data,
- false);
+ unbuffered_extend(&ub_wstate, MAIN_FORKNUM, lastblock, zerobuf.data, false);
return true;
}
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 39ef8a0b77..8824e39a91 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -38,6 +38,7 @@
#include "pgstat.h"
#include "storage/bufmgr.h"
#include "storage/bufpage.h"
+#include "storage/directmgr.h"
#include "storage/lmgr.h"
#include "storage/predicate.h"
#include "storage/procarray.h"
@@ -575,6 +576,7 @@ heapam_relation_set_new_filenode(Relation rel,
MultiXactId *minmulti)
{
SMgrRelation srel;
+ UnBufferedWriteState ub_wstate;
/*
* Initialize to the minimum XID that could put tuples in the table. We
@@ -594,15 +596,15 @@ heapam_relation_set_new_filenode(Relation rel,
*minmulti = GetOldestMultiXactId();
srel = RelationCreateStorage(*newrnode, persistence);
+ ub_wstate.smgr_rel = srel;
+ unbuffered_prep(&ub_wstate);
/*
* If required, set up an init fork for an unlogged table so that it can
- * be correctly reinitialized on restart. An immediate sync is required
- * even if the page has been logged, because the write did not go through
- * shared_buffers and therefore a concurrent checkpoint may have moved the
- * redo pointer past our xlog record. Recovery may as well remove it
- * while replaying, for example, XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE
- * record. Therefore, logging is necessary even if wal_level=minimal.
+ * be correctly reinitialized on restart.
+ * Recovery may as well remove our xlog record while replaying, for
+ * example, XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record. Therefore,
+ * logging is necessary even if wal_level=minimal.
*/
if (persistence == RELPERSISTENCE_UNLOGGED)
{
@@ -611,7 +613,7 @@ heapam_relation_set_new_filenode(Relation rel,
rel->rd_rel->relkind == RELKIND_TOASTVALUE);
smgrcreate(srel, INIT_FORKNUM, false);
log_smgrcreate(newrnode, INIT_FORKNUM);
- smgrimmedsync(srel, INIT_FORKNUM);
+ unbuffered_finish(&ub_wstate, INIT_FORKNUM);
}
smgrclose(srel);
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 5bb5ebba23..5d93850ccc 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -119,6 +119,7 @@
#include "replication/logical.h"
#include "replication/slot.h"
#include "storage/bufmgr.h"
+#include "storage/directmgr.h"
#include "storage/fd.h"
#include "storage/procarray.h"
#include "storage/smgr.h"
@@ -152,6 +153,7 @@ typedef struct RewriteStateData
HTAB *rs_old_new_tid_map; /* unmatched B tuples */
HTAB *rs_logical_mappings; /* logical remapping files */
uint32 rs_num_rewrite_mappings; /* # in memory mappings */
+ UnBufferedWriteState rs_unbuffered_wstate;
} RewriteStateData;
/*
@@ -264,6 +266,9 @@ begin_heap_rewrite(Relation old_heap, Relation new_heap, TransactionId oldest_xm
state->rs_freeze_xid = freeze_xid;
state->rs_cutoff_multi = cutoff_multi;
state->rs_cxt = rw_cxt;
+ state->rs_unbuffered_wstate.smgr_rel = RelationGetSmgr(state->rs_new_rel);
+
+ unbuffered_prep(&state->rs_unbuffered_wstate);
/* Initialize hash tables used to track update chains */
hash_ctl.keysize = sizeof(TidHashKey);
@@ -324,21 +329,12 @@ end_heap_rewrite(RewriteState state)
state->rs_buffer,
true);
- PageSetChecksumInplace(state->rs_buffer, state->rs_blockno);
-
- smgrextend(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM,
- state->rs_blockno, (char *) state->rs_buffer, true);
+ unbuffered_extend(&state->rs_unbuffered_wstate, MAIN_FORKNUM,
+ state->rs_blockno, state->rs_buffer, false);
}
- /*
- * When we WAL-logged rel pages, we must nonetheless fsync them. The
- * reason is the same as in storage.c's RelationCopyStorage(): we're
- * writing data that's not in shared buffers, and so a CHECKPOINT
- * occurring during the rewriteheap operation won't have fsync'd data we
- * wrote before the checkpoint.
- */
if (RelationNeedsWAL(state->rs_new_rel))
- smgrimmedsync(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM);
+ unbuffered_finish(&state->rs_unbuffered_wstate, MAIN_FORKNUM);
logical_end_heap_rewrite(state);
@@ -690,10 +686,8 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
* need for smgr to schedule an fsync for this write; we'll do it
* ourselves in end_heap_rewrite.
*/
- PageSetChecksumInplace(page, state->rs_blockno);
-
- smgrextend(RelationGetSmgr(state->rs_new_rel), MAIN_FORKNUM,
- state->rs_blockno, (char *) page, true);
+ unbuffered_extend(&state->rs_unbuffered_wstate, MAIN_FORKNUM,
+ state->rs_blockno, page, false);
state->rs_blockno++;
state->rs_buffer_valid = false;
diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index 9032d4758f..1a7482e267 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -92,6 +92,7 @@
#include "miscadmin.h"
#include "port/pg_bitutils.h"
#include "storage/bufmgr.h"
+#include "storage/directmgr.h"
#include "storage/lmgr.h"
#include "storage/smgr.h"
#include "utils/inval.h"
@@ -616,7 +617,9 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
BlockNumber vm_nblocks_now;
PGAlignedBlock pg;
SMgrRelation reln;
+ UnBufferedWriteState ub_wstate;
+ ub_wstate.smgr_rel = RelationGetSmgr(rel);
PageInit((Page) pg.data, BLCKSZ, 0);
/*
@@ -654,9 +657,8 @@ vm_extend(Relation rel, BlockNumber vm_nblocks)
/* Now extend the file */
while (vm_nblocks_now < vm_nblocks)
{
- PageSetChecksumInplace((Page) pg.data, vm_nblocks_now);
-
- smgrextend(reln, VISIBILITYMAP_FORKNUM, vm_nblocks_now, pg.data, false);
+ // TODO: aren't these pages empty? why checksum them
+ unbuffered_extend(&ub_wstate, VISIBILITYMAP_FORKNUM, vm_nblocks_now, (Page) pg.data, false);
vm_nblocks_now++;
}
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 13024af2fa..6ab6651420 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -29,6 +29,7 @@
#include "pgstat.h"
#include "postmaster/autovacuum.h"
#include "storage/condition_variable.h"
+#include "storage/directmgr.h"
#include "storage/indexfsm.h"
#include "storage/ipc.h"
#include "storage/lmgr.h"
@@ -151,6 +152,11 @@ void
btbuildempty(Relation index)
{
Page metapage;
+ UnBufferedWriteState wstate;
+
+ wstate.smgr_rel = RelationGetSmgr(index);
+
+ unbuffered_prep(&wstate);
/* Construct metapage. */
metapage = (Page) palloc(BLCKSZ);
@@ -163,18 +169,15 @@ btbuildempty(Relation index)
* XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record. Therefore, we need
* this even when wal_level=minimal.
*/
- PageSetChecksumInplace(metapage, BTREE_METAPAGE);
- smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, BTREE_METAPAGE,
- (char *) metapage, true);
+ unbuffered_write(&wstate, INIT_FORKNUM, BTREE_METAPAGE, metapage);
log_newpage(&RelationGetSmgr(index)->smgr_rnode.node, INIT_FORKNUM,
BTREE_METAPAGE, metapage, true);
/*
- * An immediate sync is required even if we xlog'd the page, because the
- * write did not go through shared_buffers and therefore a concurrent
- * checkpoint may have moved the redo pointer past our xlog record.
+ * Even though we xlog'd the page, a concurrent checkpoint may have moved
+ * the redo pointer past our xlog record, so we may still need to fsync.
*/
- smgrimmedsync(RelationGetSmgr(index), INIT_FORKNUM);
+ unbuffered_finish(&wstate, INIT_FORKNUM);
}
/*
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index dc220146fd..5687acd99d 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -57,6 +57,7 @@
#include "executor/instrument.h"
#include "miscadmin.h"
#include "pgstat.h"
+#include "storage/directmgr.h"
#include "storage/smgr.h"
#include "tcop/tcopprot.h" /* pgrminclude ignore */
#include "utils/rel.h"
@@ -253,6 +254,7 @@ typedef struct BTWriteState
BlockNumber btws_pages_alloced; /* # pages allocated */
BlockNumber btws_pages_written; /* # pages written out */
Page btws_zeropage; /* workspace for filling zeroes */
+ UnBufferedWriteState ub_wstate;
} BTWriteState;
@@ -560,6 +562,8 @@ _bt_leafbuild(BTSpool *btspool, BTSpool *btspool2)
wstate.heap = btspool->heap;
wstate.index = btspool->index;
+ wstate.ub_wstate.smgr_rel = RelationGetSmgr(btspool->index);
+ wstate.ub_wstate.redo = InvalidXLogRecPtr;
wstate.inskey = _bt_mkscankey(wstate.index, NULL);
/* _bt_mkscankey() won't set allequalimage without metapage */
wstate.inskey->allequalimage = _bt_allequalimage(wstate.index, true);
@@ -656,31 +660,19 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
if (!wstate->btws_zeropage)
wstate->btws_zeropage = (Page) palloc0(BLCKSZ);
/* don't set checksum for all-zero page */
- smgrextend(RelationGetSmgr(wstate->index), MAIN_FORKNUM,
- wstate->btws_pages_written++,
- (char *) wstate->btws_zeropage,
- true);
+ unbuffered_extend(&wstate->ub_wstate, MAIN_FORKNUM, wstate->btws_pages_written++, wstate->btws_zeropage, true);
}
- PageSetChecksumInplace(page, blkno);
- /*
- * Now write the page. There's no need for smgr to schedule an fsync for
- * this write; we'll do it ourselves before ending the build.
- */
+ /* Now write the page. Either we are extending the file... */
if (blkno == wstate->btws_pages_written)
{
- /* extending the file... */
- smgrextend(RelationGetSmgr(wstate->index), MAIN_FORKNUM, blkno,
- (char *) page, true);
+ unbuffered_extend(&wstate->ub_wstate, MAIN_FORKNUM, blkno, page, false);
wstate->btws_pages_written++;
}
+ /* or we are overwriting a block we zero-filled before. */
else
- {
- /* overwriting a block we zero-filled before */
- smgrwrite(RelationGetSmgr(wstate->index), MAIN_FORKNUM, blkno,
- (char *) page, true);
- }
+ unbuffered_write(&wstate->ub_wstate, MAIN_FORKNUM, blkno, page);
pfree(page);
}
@@ -1189,6 +1181,8 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
int64 tuples_done = 0;
bool deduplicate;
+
+ unbuffered_prep(&wstate->ub_wstate);
deduplicate = wstate->inskey->allequalimage && !btspool->isunique &&
BTGetDeduplicateItems(wstate->index);
@@ -1415,17 +1409,8 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
/* Close down final pages and write the metapage */
_bt_uppershutdown(wstate, state);
- /*
- * When we WAL-logged index pages, we must nonetheless fsync index files.
- * Since we're building outside shared buffers, a CHECKPOINT occurring
- * during the build has no way to flush the previously written data to
- * disk (indeed it won't know the index even exists). A crash later on
- * would replay WAL from the checkpoint, therefore it wouldn't replay our
- * earlier WAL entries. If we do not fsync those pages here, they might
- * still not be on disk when the crash occurs.
- */
if (wstate->btws_use_wal)
- smgrimmedsync(RelationGetSmgr(wstate->index), MAIN_FORKNUM);
+ unbuffered_finish(&wstate->ub_wstate, MAIN_FORKNUM);
}
/*
diff --git a/src/backend/access/spgist/spginsert.c b/src/backend/access/spgist/spginsert.c
index bfb74049d0..9fd5686b8d 100644
--- a/src/backend/access/spgist/spginsert.c
+++ b/src/backend/access/spgist/spginsert.c
@@ -25,6 +25,7 @@
#include "catalog/index.h"
#include "miscadmin.h"
#include "storage/bufmgr.h"
+#include "storage/directmgr.h"
#include "storage/smgr.h"
#include "utils/memutils.h"
#include "utils/rel.h"
@@ -156,6 +157,10 @@ void
spgbuildempty(Relation index)
{
Page page;
+ UnBufferedWriteState wstate;
+
+ wstate.smgr_rel = RelationGetSmgr(index);
+ unbuffered_prep(&wstate);
/* Construct metapage. */
page = (Page) palloc(BLCKSZ);
@@ -168,36 +173,30 @@ spgbuildempty(Relation index)
* of their existing content when the corresponding create records are
* replayed.
*/
- PageSetChecksumInplace(page, SPGIST_METAPAGE_BLKNO);
- smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_METAPAGE_BLKNO,
- (char *) page, true);
+ unbuffered_write(&wstate, INIT_FORKNUM, SPGIST_METAPAGE_BLKNO, page);
log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM,
SPGIST_METAPAGE_BLKNO, page, true);
/* Likewise for the root page. */
SpGistInitPage(page, SPGIST_LEAF);
- PageSetChecksumInplace(page, SPGIST_ROOT_BLKNO);
- smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_ROOT_BLKNO,
- (char *) page, true);
+ unbuffered_write(&wstate, INIT_FORKNUM, SPGIST_ROOT_BLKNO, page);
log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM,
SPGIST_ROOT_BLKNO, page, true);
/* Likewise for the null-tuples root page. */
SpGistInitPage(page, SPGIST_LEAF | SPGIST_NULLS);
- PageSetChecksumInplace(page, SPGIST_NULL_BLKNO);
- smgrwrite(RelationGetSmgr(index), INIT_FORKNUM, SPGIST_NULL_BLKNO,
- (char *) page, true);
+ unbuffered_write(&wstate, INIT_FORKNUM, SPGIST_NULL_BLKNO, page);
log_newpage(&(RelationGetSmgr(index))->smgr_rnode.node, INIT_FORKNUM,
SPGIST_NULL_BLKNO, page, true);
/*
- * An immediate sync is required even if we xlog'd the pages, because the
- * writes did not go through shared buffers and therefore a concurrent
- * checkpoint may have moved the redo pointer past our xlog record.
+ * Because the writes did not go through shared buffers, if a concurrent
+ * checkpoint moved the redo pointer past our xlog record, an immediate
+ * sync is required even if we xlog'd the pages.
*/
- smgrimmedsync(RelationGetSmgr(index), INIT_FORKNUM);
+ unbuffered_finish(&wstate, INIT_FORKNUM);
}
/*
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c9d4cbf3ff..a06e8d005b 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -13191,6 +13191,19 @@ CheckForStandbyTrigger(void)
return false;
}
+bool RedoRecPtrChanged(XLogRecPtr comparator_ptr)
+{
+ XLogRecPtr ptr;
+ SpinLockAcquire(&XLogCtl->info_lck);
+ ptr = XLogCtl->RedoRecPtr;
+ SpinLockRelease(&XLogCtl->info_lck);
+
+ if (RedoRecPtr < ptr)
+ RedoRecPtr = ptr;
+
+ return RedoRecPtr != comparator_ptr;
+}
+
/*
* Remove the files signaling a standby promotion request.
*/
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index 9b8075536a..05e66d0434 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -28,6 +28,7 @@
#include "catalog/storage.h"
#include "catalog/storage_xlog.h"
#include "miscadmin.h"
+#include "storage/directmgr.h"
#include "storage/freespace.h"
#include "storage/smgr.h"
#include "utils/hsearch.h"
@@ -420,6 +421,10 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
bool copying_initfork;
BlockNumber nblocks;
BlockNumber blkno;
+ UnBufferedWriteState wstate;
+
+ wstate.smgr_rel = dst;
+ unbuffered_prep(&wstate);
page = (Page) buf.data;
@@ -477,27 +482,11 @@ RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
if (use_wal)
log_newpage(&dst->smgr_rnode.node, forkNum, blkno, page, false);
- PageSetChecksumInplace(page, blkno);
-
- /*
- * Now write the page. We say skipFsync = true because there's no
- * need for smgr to schedule an fsync for this write; we'll do it
- * ourselves below.
- */
- smgrextend(dst, forkNum, blkno, buf.data, true);
+ unbuffered_extend(&wstate, forkNum, blkno, buf.data, false);
}
- /*
- * When we WAL-logged rel pages, we must nonetheless fsync them. The
- * reason is that since we're copying outside shared buffers, a CHECKPOINT
- * occurring during the copy has no way to flush the previously written
- * data to disk (indeed it won't know the new rel even exists). A crash
- * later on would replay WAL from the checkpoint, therefore it wouldn't
- * replay our earlier WAL entries. If we do not fsync those pages here,
- * they might still not be on disk when the crash occurs.
- */
if (use_wal || copying_initfork)
- smgrimmedsync(dst, forkNum);
+ unbuffered_finish(&wstate, forkNum);
}
/*
diff --git a/src/backend/storage/Makefile b/src/backend/storage/Makefile
index 8376cdfca2..501fae5f9d 100644
--- a/src/backend/storage/Makefile
+++ b/src/backend/storage/Makefile
@@ -8,6 +8,6 @@ subdir = src/backend/storage
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = buffer file freespace ipc large_object lmgr page smgr sync
+SUBDIRS = buffer direct file freespace ipc large_object lmgr page smgr sync
include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/storage/direct/Makefile b/src/backend/storage/direct/Makefile
new file mode 100644
index 0000000000..d82bbed48c
--- /dev/null
+++ b/src/backend/storage/direct/Makefile
@@ -0,0 +1,17 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+# Makefile for storage/direct
+#
+# IDENTIFICATION
+# src/backend/storage/direct/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/storage/direct
+top_builddir = ../../../..
+include $(top_builddir)/src/Makefile.global
+
+OBJS = directmgr.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/storage/direct/directmgr.c b/src/backend/storage/direct/directmgr.c
new file mode 100644
index 0000000000..d33c2a7a5f
--- /dev/null
+++ b/src/backend/storage/direct/directmgr.c
@@ -0,0 +1,57 @@
+/*-------------------------------------------------------------------------
+ *
+ * directmgr.c
+ * routines for managing unbuffered IO
+ *
+ * Portions Copyright (c) 1996-2021, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ * src/backend/storage/direct/directmgr.c
+ *
+ *-------------------------------------------------------------------------
+ */
+#include "postgres.h"
+
+
+#include "access/xlog.h"
+#include "storage/directmgr.h"
+#include "utils/rel.h"
+
+void unbuffered_prep(UnBufferedWriteState *wstate)
+{
+ wstate->redo = GetRedoRecPtr();
+}
+
+/*
+ * When writing data outside shared buffers, a concurrent CHECKPOINT can move
+ * the redo pointer past our WAL entries and won't flush our data to disk. If
+ * the database crashes before the data makes it to disk, our WAL won't be
+ * replayed and the data will be lost.
+ * Thus, if a CHECKPOINT begins between unbuffered_prep() and
+ * unbuffered_finish(), the backend must fsync the data itself.
+ */
+void unbuffered_finish(UnBufferedWriteState *wstate, ForkNumber forknum)
+{
+ if (RedoRecPtrChanged(wstate->redo))
+ smgrimmedsync(wstate->smgr_rel, forknum);
+}
+
+void
+unbuffered_write(UnBufferedWriteState *wstate, ForkNumber forknum, BlockNumber
+ blocknum, Page page)
+{
+ PageSetChecksumInplace(page, blocknum);
+ smgrwrite(wstate->smgr_rel, forknum, blocknum, (char *) page, false);
+}
+
+void
+unbuffered_extend(UnBufferedWriteState *wstate, ForkNumber forknum, BlockNumber
+ blocknum, Page page, bool empty)
+{
+ if (!empty)
+ PageSetChecksumInplace(page, blocknum);
+ smgrextend(wstate->smgr_rel, forknum, blocknum, (char *) page, false);
+}
+
diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index c88cb91f06..d31f75bd60 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -26,6 +26,7 @@
#include "access/htup_details.h"
#include "access/xlogutils.h"
#include "miscadmin.h"
+#include "storage/directmgr.h"
#include "storage/freespace.h"
#include "storage/fsm_internals.h"
#include "storage/lmgr.h"
@@ -608,6 +609,9 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
BlockNumber fsm_nblocks_now;
PGAlignedBlock pg;
SMgrRelation reln;
+ UnBufferedWriteState ub_wstate;
+
+ ub_wstate.smgr_rel = RelationGetSmgr(rel);
PageInit((Page) pg.data, BLCKSZ, 0);
@@ -647,10 +651,8 @@ fsm_extend(Relation rel, BlockNumber fsm_nblocks)
/* Extend as needed. */
while (fsm_nblocks_now < fsm_nblocks)
{
- PageSetChecksumInplace((Page) pg.data, fsm_nblocks_now);
-
- smgrextend(reln, FSM_FORKNUM, fsm_nblocks_now,
- pg.data, false);
+ // TODO: why was it checksumming all zero pages?
+ unbuffered_extend(&ub_wstate, FSM_FORKNUM, fsm_nblocks_now, (Page) pg.data, false);
fsm_nblocks_now++;
}
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index bb0c52686a..4961067f81 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -312,6 +312,7 @@ extern XLogRecPtr GetInsertRecPtr(void);
extern XLogRecPtr GetFlushRecPtr(TimeLineID *insertTLI);
extern TimeLineID GetWALInsertionTimeLine(void);
extern XLogRecPtr GetLastImportantRecPtr(void);
+extern bool RedoRecPtrChanged(XLogRecPtr comparator_ptr);
extern void RemovePromoteSignalFiles(void);
extern bool PromoteIsTriggered(void);
--
2.30.2
From f1bd40b2880d9cffeb2e313719c3fd6b4c8e5f04 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplage...@gmail.com>
Date: Mon, 10 Jan 2022 17:34:01 -0500
Subject: [PATCH v3 2/2] Use shared buffers when possible for index build
When there are not too many tuples, building the index in shared buffers
makes sense. It allows the buffer manager to handle how best to do the
IO.
---
src/backend/access/nbtree/nbtree.c | 34 ++--
src/backend/access/nbtree/nbtsort.c | 268 ++++++++++++++++++++++------
2 files changed, 225 insertions(+), 77 deletions(-)
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 6ab6651420..0714c7fdd6 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -151,33 +151,27 @@ bthandler(PG_FUNCTION_ARGS)
void
btbuildempty(Relation index)
{
+ /*
+ * Since this only writes one page, use shared buffers.
+ */
Page metapage;
- UnBufferedWriteState wstate;
-
- wstate.smgr_rel = RelationGetSmgr(index);
-
- unbuffered_prep(&wstate);
-
- /* Construct metapage. */
- metapage = (Page) palloc(BLCKSZ);
- _bt_initmetapage(metapage, P_NONE, 0, _bt_allequalimage(index, false));
+ Buffer metabuf;
/*
- * Write the page and log it. It might seem that an immediate sync would
- * be sufficient to guarantee that the file exists on disk, but recovery
- * itself might remove it while replaying, for example, an
- * XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record. Therefore, we need
- * this even when wal_level=minimal.
+ * Allocate a buffer for metapage and initialize metapage.
*/
- unbuffered_write(&wstate, INIT_FORKNUM, BTREE_METAPAGE, metapage);
- log_newpage(&RelationGetSmgr(index)->smgr_rnode.node, INIT_FORKNUM,
- BTREE_METAPAGE, metapage, true);
+ metabuf = ReadBufferExtended(index, INIT_FORKNUM, P_NEW, RBM_ZERO_AND_LOCK, NULL);
+ metapage = BufferGetPage(metabuf);
+ _bt_initmetapage(metapage, P_NONE, 0, _bt_allequalimage(index, false));
/*
- * Even though we xlog'd the page, a concurrent checkpoint may have moved
- * the redo pointer past our xlog record, so we may still need to fsync.
+ * Mark metapage buffer as dirty and XLOG it
*/
- unbuffered_finish(&wstate, INIT_FORKNUM);
+ START_CRIT_SECTION();
+ MarkBufferDirty(metabuf);
+ log_newpage_buffer(metabuf, true);
+ END_CRIT_SECTION();
+ _bt_relbuf(index, metabuf);
}
/*
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 5687acd99d..68b53e6c04 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -234,6 +234,7 @@ typedef struct BTPageState
{
Page btps_page; /* workspace for page building */
BlockNumber btps_blkno; /* block # to write this page at */
+ Buffer btps_buf; /* buffer to write this page to */
IndexTuple btps_lowkey; /* page's strict lower bound pivot tuple */
OffsetNumber btps_lastoff; /* last item offset loaded */
Size btps_lastextra; /* last item's extra posting list space */
@@ -251,10 +252,25 @@ typedef struct BTWriteState
Relation index;
BTScanInsert inskey; /* generic insertion scankey */
bool btws_use_wal; /* dump pages to WAL? */
- BlockNumber btws_pages_alloced; /* # pages allocated */
- BlockNumber btws_pages_written; /* # pages written out */
+ BlockNumber btws_pages_alloced; /* # pages allocated for index builds outside SB */
+ BlockNumber btws_pages_written; /* # pages written out for index builds outside SB */
Page btws_zeropage; /* workspace for filling zeroes */
UnBufferedWriteState ub_wstate;
+ /*
+ * Allocate a new btree page. This does not initialize the page.
+ */
+ Page (*_bt_bl_alloc_page) (struct BTWriteState *wstate, BlockNumber
+ *blockno, Buffer *buf);
+ /*
+ * Emit a completed btree page, and release the working storage.
+ */
+ void (*_bt_blwritepage) (struct BTWriteState *wstate, Page page,
+ BlockNumber blkno, Buffer buf);
+
+ void (*_bt_bl_unbuffered_prep) (UnBufferedWriteState *wstate);
+
+ void (*_bt_bl_unbuffered_finish) (UnBufferedWriteState *wstate, ForkNumber
+ forknum);
} BTWriteState;
@@ -263,10 +279,22 @@ static double _bt_spools_heapscan(Relation heap, Relation index,
static void _bt_spooldestroy(BTSpool *btspool);
static void _bt_spool(BTSpool *btspool, ItemPointer self,
Datum *values, bool *isnull);
-static void _bt_leafbuild(BTSpool *btspool, BTSpool *btspool2);
+static void _bt_leafbuild(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2);
static void _bt_build_callback(Relation index, ItemPointer tid, Datum *values,
bool *isnull, bool tupleIsAlive, void *state);
-static Page _bt_blnewpage(uint32 level);
+
+static Page
+_bt_bl_alloc_page_direct(BTWriteState *wstate, BlockNumber *blockno, Buffer *buf);
+static Page
+_bt_bl_alloc_page_shared(BTWriteState *wstate, BlockNumber *blockno, Buffer *buf);
+
+static Page _bt_blnewpage(uint32 level, Page page);
+
+static void
+_bt_blwritepage_direct(BTWriteState *wstate, Page page, BlockNumber blkno, Buffer buf);
+static void
+_bt_blwritepage_shared(BTWriteState *wstate, Page page, BlockNumber blkno, Buffer buf);
+
static BTPageState *_bt_pagestate(BTWriteState *wstate, uint32 level);
static void _bt_slideleft(Page rightmostpage);
static void _bt_sortaddtup(Page page, Size itemsize,
@@ -277,9 +305,10 @@ static void _bt_buildadd(BTWriteState *wstate, BTPageState *state,
static void _bt_sort_dedup_finish_pending(BTWriteState *wstate,
BTPageState *state,
BTDedupState dstate);
-static void _bt_uppershutdown(BTWriteState *wstate, BTPageState *state);
static void _bt_load(BTWriteState *wstate,
BTSpool *btspool, BTSpool *btspool2);
+static void _bt_uppershutdown(BTWriteState *wstate, BTPageState *state, Buffer
+ metabuf, Page metapage);
static void _bt_begin_parallel(BTBuildState *buildstate, bool isconcurrent,
int request);
static void _bt_end_parallel(BTLeader *btleader);
@@ -292,6 +321,21 @@ static void _bt_parallel_scan_and_sort(BTSpool *btspool, BTSpool *btspool2,
Sharedsort *sharedsort2, int sortmem,
bool progress);
+#define BT_BUILD_SB_THRESHOLD 1024
+
+static const BTWriteState wstate_shared = {
+ ._bt_bl_alloc_page = _bt_bl_alloc_page_shared,
+ ._bt_blwritepage = _bt_blwritepage_shared,
+ ._bt_bl_unbuffered_prep = NULL,
+ ._bt_bl_unbuffered_finish = NULL,
+};
+
+static const BTWriteState wstate_direct = {
+ ._bt_bl_alloc_page = _bt_bl_alloc_page_direct,
+ ._bt_blwritepage = _bt_blwritepage_direct,
+ ._bt_bl_unbuffered_prep = unbuffered_prep,
+ ._bt_bl_unbuffered_finish = unbuffered_finish,
+};
/*
* btbuild() -- build a new btree index.
@@ -301,6 +345,7 @@ btbuild(Relation heap, Relation index, IndexInfo *indexInfo)
{
IndexBuildResult *result;
BTBuildState buildstate;
+ BTWriteState writestate;
double reltuples;
#ifdef BTREE_BUILD_STATS
@@ -330,8 +375,12 @@ btbuild(Relation heap, Relation index, IndexInfo *indexInfo)
* Finish the build by (1) completing the sort of the spool file, (2)
* inserting the sorted tuples into btree pages and (3) building the upper
* levels. Finally, it may also be necessary to end use of parallelism.
+ *
+ * Don't use shared buffers if the number of tuples is too large.
*/
- _bt_leafbuild(buildstate.spool, buildstate.spool2);
+ writestate = reltuples < BT_BUILD_SB_THRESHOLD ? wstate_shared : wstate_direct;
+
+ _bt_leafbuild(&writestate, buildstate.spool, buildstate.spool2);
_bt_spooldestroy(buildstate.spool);
if (buildstate.spool2)
_bt_spooldestroy(buildstate.spool2);
@@ -537,10 +586,8 @@ _bt_spool(BTSpool *btspool, ItemPointer self, Datum *values, bool *isnull)
* create an entire btree.
*/
static void
-_bt_leafbuild(BTSpool *btspool, BTSpool *btspool2)
+_bt_leafbuild(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
{
- BTWriteState wstate;
-
#ifdef BTREE_BUILD_STATS
if (log_btree_build_stats)
{
@@ -560,23 +607,38 @@ _bt_leafbuild(BTSpool *btspool, BTSpool *btspool2)
tuplesort_performsort(btspool2->sortstate);
}
- wstate.heap = btspool->heap;
- wstate.index = btspool->index;
- wstate.ub_wstate.smgr_rel = RelationGetSmgr(btspool->index);
- wstate.ub_wstate.redo = InvalidXLogRecPtr;
- wstate.inskey = _bt_mkscankey(wstate.index, NULL);
- /* _bt_mkscankey() won't set allequalimage without metapage */
- wstate.inskey->allequalimage = _bt_allequalimage(wstate.index, true);
- wstate.btws_use_wal = RelationNeedsWAL(wstate.index);
+ wstate->heap = btspool->heap;
+ wstate->index = btspool->index;
+ wstate->ub_wstate.smgr_rel = RelationGetSmgr(btspool->index);
+ wstate->ub_wstate.redo = InvalidXLogRecPtr;
+ wstate->inskey = _bt_mkscankey(wstate->index, NULL);
+ /* _bt-mkscankey() won't set allequalimage without metapage */
+ wstate->inskey->allequalimage = _bt_allequalimage(wstate->index, true);
+ wstate->btws_use_wal = RelationNeedsWAL(wstate->index);
/* reserve the metapage */
- wstate.btws_pages_alloced = BTREE_METAPAGE + 1;
- wstate.btws_pages_written = 0;
- wstate.btws_zeropage = NULL; /* until needed */
+ wstate->btws_pages_alloced = 0;
+ wstate->btws_pages_written = 0;
+ wstate->btws_zeropage = NULL;
pgstat_progress_update_param(PROGRESS_CREATEIDX_SUBPHASE,
PROGRESS_BTREE_PHASE_LEAF_LOAD);
- _bt_load(&wstate, btspool, btspool2);
+
+ /*
+ * If not using shared buffers, save the redo pointer location in case a
+ * checkpoint begins during the index build.
+ */
+ if (wstate->_bt_bl_unbuffered_prep && wstate->btws_use_wal)
+ wstate->_bt_bl_unbuffered_prep(&wstate->ub_wstate);
+
+ _bt_load(wstate, btspool, btspool2);
+
+ /*
+ * If not using shared buffers, check if backend must fsync the page
+ * itself.
+ */
+ if (wstate->_bt_bl_unbuffered_finish && wstate->btws_use_wal)
+ wstate->_bt_bl_unbuffered_finish(&wstate->ub_wstate, MAIN_FORKNUM);
}
/*
@@ -609,15 +671,15 @@ _bt_build_callback(Relation index,
}
/*
- * allocate workspace for a new, clean btree page, not linked to any siblings.
+ * Set up workspace for a new, clean btree page, not linked to any siblings.
+ * Caller must allocate the passed in page.
*/
static Page
-_bt_blnewpage(uint32 level)
+_bt_blnewpage(uint32 level, Page page)
{
- Page page;
BTPageOpaque opaque;
- page = (Page) palloc(BLCKSZ);
+ Assert(page);
/* Zero the page and set up standard page header info */
_bt_pageinit(page, BLCKSZ);
@@ -635,18 +697,18 @@ _bt_blnewpage(uint32 level)
return page;
}
-/*
- * emit a completed btree page, and release the working storage.
- */
static void
-_bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
+_bt_blwritepage_direct(BTWriteState *wstate, Page page, BlockNumber blkno, Buffer buf)
{
- /* XLOG stuff */
+ /*
+ * Indexes built outside shared buffers must XLOG the page, issue the page
+ * write request, and take care of fsync'ing the page to the device if a
+ * checkpoint began after the beginning of the index build.
+ *
+ * Use the XLOG_FPI record type for this.
+ */
if (wstate->btws_use_wal)
- {
- /* We use the XLOG_FPI record type for this */
log_newpage(&wstate->index->rd_node, MAIN_FORKNUM, blkno, page, true);
- }
/*
* If we have to write pages nonsequentially, fill in the space with
@@ -660,7 +722,8 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
if (!wstate->btws_zeropage)
wstate->btws_zeropage = (Page) palloc0(BLCKSZ);
/* don't set checksum for all-zero page */
- unbuffered_extend(&wstate->ub_wstate, MAIN_FORKNUM, wstate->btws_pages_written++, wstate->btws_zeropage, true);
+ unbuffered_extend(&wstate->ub_wstate, MAIN_FORKNUM,
+ wstate->btws_pages_written++, wstate->btws_zeropage, true);
}
@@ -677,6 +740,61 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
pfree(page);
}
+static void
+_bt_blwritepage_shared(BTWriteState *wstate, Page page, BlockNumber blkno, Buffer buf)
+{
+ /*
+ * Indexes built in shared buffers need only to mark the buffer as dirty
+ * and XLOG it.
+ */
+ Assert(buf);
+ START_CRIT_SECTION();
+ MarkBufferDirty(buf);
+ if (wstate->btws_use_wal)
+ log_newpage_buffer(buf, true);
+ END_CRIT_SECTION();
+ _bt_relbuf(wstate->index, buf);
+}
+
+static Page
+_bt_bl_alloc_page_direct(BTWriteState *wstate, BlockNumber *blockno, Buffer *buf)
+{
+ /* buf is only used when using shared buffers, so set it to InvalidBuffer */
+ *buf = InvalidBuffer;
+
+ /*
+ * Assign block number for the page.
+ * This will be used to link to sibling page(s) later and, if this is the
+ * initial page in the level, saved in the BTPageState
+ */
+ *blockno = wstate->btws_pages_alloced++;
+
+ /* now allocate and set up the new page */
+ return palloc(BLCKSZ);
+}
+
+static Page
+_bt_bl_alloc_page_shared(BTWriteState *wstate, BlockNumber *blockno, Buffer *buf)
+{
+ /*
+ * Find a shared buffer for the page. Pass mode RBM_ZERO_AND_LOCK to get an
+ * exclusive lock on the buffer content. No lock on the relation as a whole
+ * is needed (as in LockRelationForExtension()) because the initial index
+ * build is not yet complete.
+ */
+ *buf = ReadBufferExtended(wstate->index, MAIN_FORKNUM, P_NEW,
+ RBM_ZERO_AND_LOCK, NULL);
+
+ /*
+ * bufmgr will assign a block number for the new page.
+ * This will be used to link to sibling page(s) later and, if this is the
+ * initial page in the level, saved in the BTPageState
+ */
+ *blockno = BufferGetBlockNumber(*buf);
+
+ return BufferGetPage(*buf);
+}
+
/*
* allocate and initialize a new BTPageState. the returned structure
* is suitable for immediate use by _bt_buildadd.
@@ -684,13 +802,20 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno)
static BTPageState *
_bt_pagestate(BTWriteState *wstate, uint32 level)
{
+ Buffer buf;
+ BlockNumber blockno;
+
BTPageState *state = (BTPageState *) palloc0(sizeof(BTPageState));
- /* create initial page for level */
- state->btps_page = _bt_blnewpage(level);
+ /*
+ * Allocate and initialize initial page for the level, and if using shared
+ * buffers, extend the relation and allocate a shared buffer for the block.
+ */
+ state->btps_page = _bt_blnewpage(level, wstate->_bt_bl_alloc_page(wstate,
+ &blockno, &buf));
- /* and assign it a page position */
- state->btps_blkno = wstate->btws_pages_alloced++;
+ state->btps_blkno = blockno;
+ state->btps_buf = buf;
state->btps_lowkey = NULL;
/* initialize lastoff so first item goes into P_FIRSTKEY */
@@ -825,6 +950,7 @@ _bt_buildadd(BTWriteState *wstate, BTPageState *state, IndexTuple itup,
{
Page npage;
BlockNumber nblkno;
+ Buffer nbuf;
OffsetNumber last_off;
Size last_truncextra;
Size pgspc;
@@ -839,6 +965,7 @@ _bt_buildadd(BTWriteState *wstate, BTPageState *state, IndexTuple itup,
npage = state->btps_page;
nblkno = state->btps_blkno;
+ nbuf = state->btps_buf;
last_off = state->btps_lastoff;
last_truncextra = state->btps_lastextra;
state->btps_lastextra = truncextra;
@@ -895,15 +1022,14 @@ _bt_buildadd(BTWriteState *wstate, BTPageState *state, IndexTuple itup,
*/
Page opage = npage;
BlockNumber oblkno = nblkno;
+ Buffer obuf = nbuf;
ItemId ii;
ItemId hii;
IndexTuple oitup;
- /* Create new page of same level */
- npage = _bt_blnewpage(state->btps_level);
-
- /* and assign it a page position */
- nblkno = wstate->btws_pages_alloced++;
+ /* Create and initialize a new page of same level */
+ npage = _bt_blnewpage(state->btps_level,
+ wstate->_bt_bl_alloc_page(wstate, &nblkno, &nbuf));
/*
* We copy the last item on the page into the new page, and then
@@ -1013,9 +1139,12 @@ _bt_buildadd(BTWriteState *wstate, BTPageState *state, IndexTuple itup,
/*
* Write out the old page. We never need to touch it again, so we can
- * free the opage workspace too.
+ * free the opage workspace too. obuf has been released and is no longer
+ * valid.
*/
- _bt_blwritepage(wstate, opage, oblkno);
+ wstate->_bt_blwritepage(wstate, opage, oblkno, obuf);
+ obuf = InvalidBuffer;
+ opage = NULL;
/*
* Reset last_off to point to new page
@@ -1050,6 +1179,7 @@ _bt_buildadd(BTWriteState *wstate, BTPageState *state, IndexTuple itup,
state->btps_page = npage;
state->btps_blkno = nblkno;
+ state->btps_buf = nbuf;
state->btps_lastoff = last_off;
}
@@ -1095,12 +1225,12 @@ _bt_sort_dedup_finish_pending(BTWriteState *wstate, BTPageState *state,
* Finish writing out the completed btree.
*/
static void
-_bt_uppershutdown(BTWriteState *wstate, BTPageState *state)
+_bt_uppershutdown(BTWriteState *wstate, BTPageState *state, Buffer metabuf,
+ Page metapage)
{
BTPageState *s;
BlockNumber rootblkno = P_NONE;
uint32 rootlevel = 0;
- Page metapage;
/*
* Each iteration of this loop completes one more level of the tree.
@@ -1146,20 +1276,24 @@ _bt_uppershutdown(BTWriteState *wstate, BTPageState *state)
* back one slot. Then we can dump out the page.
*/
_bt_slideleft(s->btps_page);
- _bt_blwritepage(wstate, s->btps_page, s->btps_blkno);
+ wstate->_bt_blwritepage(wstate, s->btps_page, s->btps_blkno, s->btps_buf);
+ s->btps_buf = InvalidBuffer;
s->btps_page = NULL; /* writepage freed the workspace */
}
/*
- * As the last step in the process, construct the metapage and make it
+ * As the last step in the process, initialize the metapage and make it
* point to the new root (unless we had no data at all, in which case it's
* set to point to "P_NONE"). This changes the index to the "valid" state
* by filling in a valid magic number in the metapage.
+ * After this, metapage will have been freed or invalid and metabuf, if ever
+ * valid, will have been released.
*/
- metapage = (Page) palloc(BLCKSZ);
_bt_initmetapage(metapage, rootblkno, rootlevel,
wstate->inskey->allequalimage);
- _bt_blwritepage(wstate, metapage, BTREE_METAPAGE);
+ wstate->_bt_blwritepage(wstate, metapage, BTREE_METAPAGE, metabuf);
+ metabuf = InvalidBuffer;
+ metapage = NULL;
}
/*
@@ -1169,6 +1303,10 @@ _bt_uppershutdown(BTWriteState *wstate, BTPageState *state)
static void
_bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
{
+ Page metapage;
+ BlockNumber metablkno;
+ Buffer metabuf;
+
BTPageState *state = NULL;
bool merge = (btspool2 != NULL);
IndexTuple itup,
@@ -1181,11 +1319,30 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
int64 tuples_done = 0;
bool deduplicate;
-
- unbuffered_prep(&wstate->ub_wstate);
deduplicate = wstate->inskey->allequalimage && !btspool->isunique &&
BTGetDeduplicateItems(wstate->index);
+ /*
+ * Reserve block 0 for the metapage up front.
+ *
+ * When using the shared buffers API it is easier to allocate the buffer
+ * for block 0 first instead of trying skip block 0 and allocate it at the
+ * end of index build.
+ *
+ * When not using the shared buffers API, there is no harm in allocating
+ * the metapage first. When block 1 is written, the direct writepage
+ * function will zero-fill block 0. When writing out the metapage at the
+ * end of index build, it will overwrite that block 0.
+ *
+ * The metapage will be initialized and written out at the end of the index
+ * build when all of the information needed to do so is available.
+ *
+ * The block number will always be BTREE_METAPAGE, so the metablkno
+ * variable is unused and only created to avoid a special case in the
+ * direct alloc function.
+ */
+ metapage = wstate->_bt_bl_alloc_page(wstate, &metablkno, &metabuf);
+
if (merge)
{
/*
@@ -1407,10 +1564,7 @@ _bt_load(BTWriteState *wstate, BTSpool *btspool, BTSpool *btspool2)
}
/* Close down final pages and write the metapage */
- _bt_uppershutdown(wstate, state);
-
- if (wstate->btws_use_wal)
- unbuffered_finish(&wstate->ub_wstate, MAIN_FORKNUM);
+ _bt_uppershutdown(wstate, state, metabuf, metapage);
}
/*
--
2.30.2
