pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: smgrzeroextend clarification
Date: Tue, 16 May 2023 20:40:01 +0200
Message-ID: <c9e36e90-3674-e256-4bec-9971cacd4ca2@enterprisedb.com> (raw)
In-Reply-To: <20230510181049.ad5vw6kijqd2y274@awork3.anarazel.de>
References: <22fed8ba-01c3-2008-a256-4ea912d68fab@enterprisedb.com>
	<20230510181049.ad5vw6kijqd2y274@awork3.anarazel.de>

On 10.05.23 20:10, Andres Freund wrote:
>> So if you want to understand what smgrzeroextend() does, you need to
>> mentally combine the documentation of three different functions.  Could we
>> write documentation for each function that stands on its own?  And document
>> the function arguments, like what does blocknum and nblocks mean?
> I guess it couldn't hurt. But if we go down that route, we basically need to
> rewrite all the function headers in smgr.c, I think.

I took a stab at this, going through the function comments in smgr.c and 
md.c and try to make some things easier to follow.

- Took at out the weird leading tabs in the older comments.

- Rephrased some comments so that smgr.c is more like an API 
documentation and md.c documents what that particular instance of that 
API does.

- Move the *extend and *zeroextend functions to a more sensible place 
among all the functions.  Especially since *write and *extend are very 
similar, it makes sense to have them close together.  This way, it's 
also easier to follow "this function is like that function" comments.

- Also moved mdwriteback(), which was in some completely odd place.

- Added comments for function arguments that were previously not documented.

- Reworded the comments for *extend and *zeroextend to make more sense 
relative to each other.

- I left this for smgrzeroextend():

FIXME: why both blocknum and nblocks

Like, what happens when blocknum is not the block right after the last 
existing block?  Do you get an implicit POSIX hole between the end of 
the file and the specified block and then an explicit hole for the next 
nblocks?  We should be clear here what the designer of this API intended.


The name smgrzeroextend is not great.  The "zero" isn't what 
distinguishes it from smgrextend.

Attachments:

0001-WIP-Improve-smgr-source-code-comments.patch (text/plain, 38.0K) download From fadd72afcf78a55a2cfd32217b317f17a9147962 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter@eisentraut.org> Date: Tue, 16 May 2023 16:10:48 +0200 Subject: [PATCH] WIP: Improve smgr source code comments --- src/backend/storage/smgr/md.c | 501 ++++++++++++++++---------------- src/backend/storage/smgr/smgr.c | 251 ++++++++-------- src/include/storage/md.h | 8 +- src/include/storage/smgr.h | 8 +- 4 files changed, 382 insertions(+), 386 deletions(-) diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c index e982a8dd7f..4115a24b3f 100644 --- a/src/backend/storage/smgr/md.c +++ b/src/backend/storage/smgr/md.c @@ -154,7 +154,7 @@ _mdfd_open_flags(void) } /* - * mdinit() -- Initialize private state for magnetic disk storage manager. + * mdinit() -- Initialize private state for magnetic disk storage manager. */ void mdinit(void) @@ -165,7 +165,7 @@ mdinit(void) } /* - * mdexists() -- Does the physical file exist? + * mdexists() -- Does the physical file exist? * * Note: this will return true for lingering files, with pending deletions */ @@ -184,7 +184,7 @@ mdexists(SMgrRelation reln, ForkNumber forknum) } /* - * mdcreate() -- Create a new relation on magnetic disk. + * mdcreate() -- Create a new relation on magnetic disk. * * If isRedo is true, it's okay for the relation to exist already. */ @@ -242,7 +242,7 @@ mdcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo) } /* - * mdunlink() -- Unlink a relation. + * mdunlink() -- Unlink a relation. * * Note that we're passed a RelFileLocatorBackend --- by the time this is called, * there won't be an SMgrRelation hashtable entry anymore. @@ -447,183 +447,7 @@ mdunlinkfork(RelFileLocatorBackend rlocator, ForkNumber forknum, bool isRedo) } /* - * mdextend() -- Add a block to the specified relation. - * - * The semantics are nearly the same as mdwrite(): write at the - * specified position. However, this is to be used for the case of - * extending a relation (i.e., blocknum is at or beyond the current - * EOF). Note that we assume writing a block beyond current EOF - * causes intervening file space to become filled with zeroes. - */ -void -mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, - const void *buffer, bool skipFsync) -{ - off_t seekpos; - int nbytes; - MdfdVec *v; - - /* If this build supports direct I/O, the buffer must be I/O aligned. */ - if (PG_O_DIRECT != 0 && PG_IO_ALIGN_SIZE <= BLCKSZ) - Assert((uintptr_t) buffer == TYPEALIGN(PG_IO_ALIGN_SIZE, buffer)); - - /* This assert is too expensive to have on normally ... */ -#ifdef CHECK_WRITE_VS_EXTEND - Assert(blocknum >= mdnblocks(reln, forknum)); -#endif - - /* - * If a relation manages to grow to 2^32-1 blocks, refuse to extend it any - * more --- we mustn't create a block whose number actually is - * InvalidBlockNumber. (Note that this failure should be unreachable - * because of upstream checks in bufmgr.c.) - */ - if (blocknum == InvalidBlockNumber) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("cannot extend file \"%s\" beyond %u blocks", - relpath(reln->smgr_rlocator, forknum), - InvalidBlockNumber))); - - v = _mdfd_getseg(reln, forknum, blocknum, skipFsync, EXTENSION_CREATE); - - seekpos = (off_t) BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)); - - Assert(seekpos < (off_t) BLCKSZ * RELSEG_SIZE); - - if ((nbytes = FileWrite(v->mdfd_vfd, buffer, BLCKSZ, seekpos, WAIT_EVENT_DATA_FILE_EXTEND)) != BLCKSZ) - { - if (nbytes < 0) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not extend file \"%s\": %m", - FilePathName(v->mdfd_vfd)), - errhint("Check free disk space."))); - /* short write: complain appropriately */ - ereport(ERROR, - (errcode(ERRCODE_DISK_FULL), - errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u", - FilePathName(v->mdfd_vfd), - nbytes, BLCKSZ, blocknum), - errhint("Check free disk space."))); - } - - if (!skipFsync && !SmgrIsTemp(reln)) - register_dirty_segment(reln, forknum, v); - - Assert(_mdnblocks(reln, forknum, v) <= ((BlockNumber) RELSEG_SIZE)); -} - -/* - * mdzeroextend() -- Add new zeroed out blocks to the specified relation. - * - * Similar to mdextend(), except the relation can be extended by multiple - * blocks at once and the added blocks will be filled with zeroes. - */ -void -mdzeroextend(SMgrRelation reln, ForkNumber forknum, - BlockNumber blocknum, int nblocks, bool skipFsync) -{ - MdfdVec *v; - BlockNumber curblocknum = blocknum; - int remblocks = nblocks; - - Assert(nblocks > 0); - - /* This assert is too expensive to have on normally ... */ -#ifdef CHECK_WRITE_VS_EXTEND - Assert(blocknum >= mdnblocks(reln, forknum)); -#endif - - /* - * If a relation manages to grow to 2^32-1 blocks, refuse to extend it any - * more --- we mustn't create a block whose number actually is - * InvalidBlockNumber or larger. - */ - if ((uint64) blocknum + nblocks >= (uint64) InvalidBlockNumber) - ereport(ERROR, - (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), - errmsg("cannot extend file \"%s\" beyond %u blocks", - relpath(reln->smgr_rlocator, forknum), - InvalidBlockNumber))); - - while (remblocks > 0) - { - BlockNumber segstartblock = curblocknum % ((BlockNumber) RELSEG_SIZE); - off_t seekpos = (off_t) BLCKSZ * segstartblock; - int numblocks; - - if (segstartblock + remblocks > RELSEG_SIZE) - numblocks = RELSEG_SIZE - segstartblock; - else - numblocks = remblocks; - - v = _mdfd_getseg(reln, forknum, curblocknum, skipFsync, EXTENSION_CREATE); - - Assert(segstartblock < RELSEG_SIZE); - Assert(segstartblock + numblocks <= RELSEG_SIZE); - - /* - * If available and useful, use posix_fallocate() (via FileAllocate()) - * to extend the relation. That's often more efficient than using - * write(), as it commonly won't cause the kernel to allocate page - * cache space for the extended pages. - * - * However, we don't use FileAllocate() for small extensions, as it - * defeats delayed allocation on some filesystems. Not clear where - * that decision should be made though? For now just use a cutoff of - * 8, anything between 4 and 8 worked OK in some local testing. - */ - if (numblocks > 8) - { - int ret; - - ret = FileFallocate(v->mdfd_vfd, - seekpos, (off_t) BLCKSZ * numblocks, - WAIT_EVENT_DATA_FILE_EXTEND); - if (ret != 0) - { - ereport(ERROR, - errcode_for_file_access(), - errmsg("could not extend file \"%s\" with FileFallocate(): %m", - FilePathName(v->mdfd_vfd)), - errhint("Check free disk space.")); - } - } - else - { - int ret; - - /* - * Even if we don't want to use fallocate, we can still extend a - * bit more efficiently than writing each 8kB block individually. - * pg_pwrite_zeros() (via FileZero()) uses - * pg_pwritev_with_retry() to avoid multiple writes or needing a - * zeroed buffer for the whole length of the extension. - */ - ret = FileZero(v->mdfd_vfd, - seekpos, (off_t) BLCKSZ * numblocks, - WAIT_EVENT_DATA_FILE_EXTEND); - if (ret < 0) - ereport(ERROR, - errcode_for_file_access(), - errmsg("could not extend file \"%s\": %m", - FilePathName(v->mdfd_vfd)), - errhint("Check free disk space.")); - } - - if (!skipFsync && !SmgrIsTemp(reln)) - register_dirty_segment(reln, forknum, v); - - Assert(_mdnblocks(reln, forknum, v) <= ((BlockNumber) RELSEG_SIZE)); - - remblocks -= numblocks; - curblocknum += numblocks; - } -} - -/* - * mdopenfork() -- Open one fork of the specified relation. + * mdopenfork() -- Open one fork of the specified relation. * * Note we only open the first segment, when there are multiple segments. * @@ -673,7 +497,7 @@ mdopenfork(SMgrRelation reln, ForkNumber forknum, int behavior) } /* - * mdopen() -- Initialize newly-opened relation. + * mdopen() -- Initialize newly-opened relation. */ void mdopen(SMgrRelation reln) @@ -684,7 +508,7 @@ mdopen(SMgrRelation reln) } /* - * mdclose() -- Close the specified relation, if it isn't closed already. + * mdclose() -- Close the specified relation, if it isn't closed already. */ void mdclose(SMgrRelation reln, ForkNumber forknum) @@ -707,7 +531,7 @@ mdclose(SMgrRelation reln, ForkNumber forknum) } /* - * mdprefetch() -- Initiate asynchronous read of the specified block of a relation + * mdprefetch() -- Initiate asynchronous read of the specified block of a relation */ bool mdprefetch(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum) @@ -734,64 +558,7 @@ mdprefetch(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum) } /* - * mdwriteback() -- Tell the kernel to write pages back to storage. - * - * This accepts a range of blocks because flushing several pages at once is - * considerably more efficient than doing so individually. - */ -void -mdwriteback(SMgrRelation reln, ForkNumber forknum, - BlockNumber blocknum, BlockNumber nblocks) -{ - Assert((io_direct_flags & IO_DIRECT_DATA) == 0); - - /* - * Issue flush requests in as few requests as possible; have to split at - * segment boundaries though, since those are actually separate files. - */ - while (nblocks > 0) - { - BlockNumber nflush = nblocks; - off_t seekpos; - MdfdVec *v; - int segnum_start, - segnum_end; - - v = _mdfd_getseg(reln, forknum, blocknum, true /* not used */ , - EXTENSION_DONT_OPEN); - - /* - * We might be flushing buffers of already removed relations, that's - * ok, just ignore that case. If the segment file wasn't open already - * (ie from a recent mdwrite()), then we don't want to re-open it, to - * avoid a race with PROCSIGNAL_BARRIER_SMGRRELEASE that might leave - * us with a descriptor to a file that is about to be unlinked. - */ - if (!v) - return; - - /* compute offset inside the current segment */ - segnum_start = blocknum / RELSEG_SIZE; - - /* compute number of desired writes within the current segment */ - segnum_end = (blocknum + nblocks - 1) / RELSEG_SIZE; - if (segnum_start != segnum_end) - nflush = RELSEG_SIZE - (blocknum % ((BlockNumber) RELSEG_SIZE)); - - Assert(nflush >= 1); - Assert(nflush <= nblocks); - - seekpos = (off_t) BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)); - - FileWriteback(v->mdfd_vfd, seekpos, (off_t) BLCKSZ * nflush, WAIT_EVENT_DATA_FILE_FLUSH); - - nblocks -= nflush; - blocknum += nflush; - } -} - -/* - * mdread() -- Read the specified block from a relation. + * mdread() -- Read the specified block from a relation. */ void mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, @@ -856,11 +623,7 @@ mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, } /* - * mdwrite() -- Write the supplied block at the appropriate location. - * - * This is to be used only for updating already-existing blocks of a - * relation (ie, those before the current EOF). To extend a relation, - * use mdextend(). + * mdwrite() -- Write the supplied block at the appropriate location. */ void mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, @@ -924,12 +687,242 @@ mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, } /* - * mdnblocks() -- Get the number of blocks stored in a relation. + * mdextend() -- Add a block to the specified relation. + * + * This is to be used for the case of extending a relation (i.e., blocknum is + * at or beyond the current EOF). Note that writing a block beyond current + * EOF must cause the intervening file space to become filled with zeroes. + * The POSIX file system APIs do that automatically, so we don't need to do + * anything about that. + */ +void +mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, + const void *buffer, bool skipFsync) +{ + off_t seekpos; + int nbytes; + MdfdVec *v; + + /* If this build supports direct I/O, the buffer must be I/O aligned. */ + if (PG_O_DIRECT != 0 && PG_IO_ALIGN_SIZE <= BLCKSZ) + Assert((uintptr_t) buffer == TYPEALIGN(PG_IO_ALIGN_SIZE, buffer)); + + /* This assert is too expensive to have on normally ... */ +#ifdef CHECK_WRITE_VS_EXTEND + Assert(blocknum >= mdnblocks(reln, forknum)); +#endif + + /* + * If a relation manages to grow to 2^32-1 blocks, refuse to extend it any + * more --- we mustn't create a block whose number actually is + * InvalidBlockNumber. (Note that this failure should be unreachable + * because of upstream checks in bufmgr.c.) + */ + if (blocknum == InvalidBlockNumber) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("cannot extend file \"%s\" beyond %u blocks", + relpath(reln->smgr_rlocator, forknum), + InvalidBlockNumber))); + + v = _mdfd_getseg(reln, forknum, blocknum, skipFsync, EXTENSION_CREATE); + + seekpos = (off_t) BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)); + + Assert(seekpos < (off_t) BLCKSZ * RELSEG_SIZE); + + if ((nbytes = FileWrite(v->mdfd_vfd, buffer, BLCKSZ, seekpos, WAIT_EVENT_DATA_FILE_EXTEND)) != BLCKSZ) + { + if (nbytes < 0) + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not extend file \"%s\": %m", + FilePathName(v->mdfd_vfd)), + errhint("Check free disk space."))); + /* short write: complain appropriately */ + ereport(ERROR, + (errcode(ERRCODE_DISK_FULL), + errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u", + FilePathName(v->mdfd_vfd), + nbytes, BLCKSZ, blocknum), + errhint("Check free disk space."))); + } + + if (!skipFsync && !SmgrIsTemp(reln)) + register_dirty_segment(reln, forknum, v); + + Assert(_mdnblocks(reln, forknum, v) <= ((BlockNumber) RELSEG_SIZE)); +} + +/* + * mdzeroextend() -- Add new zeroed out blocks to the specified relation. + */ +void +mdzeroextend(SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, int nblocks, bool skipFsync) +{ + MdfdVec *v; + BlockNumber curblocknum = blocknum; + int remblocks = nblocks; + + Assert(nblocks > 0); + + /* This assert is too expensive to have on normally ... */ +#ifdef CHECK_WRITE_VS_EXTEND + Assert(blocknum >= mdnblocks(reln, forknum)); +#endif + + /* + * If a relation manages to grow to 2^32-1 blocks, refuse to extend it any + * more --- we mustn't create a block whose number actually is + * InvalidBlockNumber or larger. + */ + if ((uint64) blocknum + nblocks >= (uint64) InvalidBlockNumber) + ereport(ERROR, + (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), + errmsg("cannot extend file \"%s\" beyond %u blocks", + relpath(reln->smgr_rlocator, forknum), + InvalidBlockNumber))); + + while (remblocks > 0) + { + BlockNumber segstartblock = curblocknum % ((BlockNumber) RELSEG_SIZE); + off_t seekpos = (off_t) BLCKSZ * segstartblock; + int numblocks; + + if (segstartblock + remblocks > RELSEG_SIZE) + numblocks = RELSEG_SIZE - segstartblock; + else + numblocks = remblocks; + + v = _mdfd_getseg(reln, forknum, curblocknum, skipFsync, EXTENSION_CREATE); + + Assert(segstartblock < RELSEG_SIZE); + Assert(segstartblock + numblocks <= RELSEG_SIZE); + + /* + * If available and useful, use posix_fallocate() (via FileAllocate()) + * to extend the relation. That's often more efficient than using + * write(), as it commonly won't cause the kernel to allocate page + * cache space for the extended pages. + * + * However, we don't use FileAllocate() for small extensions, as it + * defeats delayed allocation on some filesystems. Not clear where + * that decision should be made though? For now just use a cutoff of + * 8, anything between 4 and 8 worked OK in some local testing. + */ + if (numblocks > 8) + { + int ret; + + ret = FileFallocate(v->mdfd_vfd, + seekpos, (off_t) BLCKSZ * numblocks, + WAIT_EVENT_DATA_FILE_EXTEND); + if (ret != 0) + { + ereport(ERROR, + errcode_for_file_access(), + errmsg("could not extend file \"%s\" with FileFallocate(): %m", + FilePathName(v->mdfd_vfd)), + errhint("Check free disk space.")); + } + } + else + { + int ret; + + /* + * Even if we don't want to use fallocate, we can still extend a + * bit more efficiently than writing each 8kB block individually. + * pg_pwrite_zeros() (via FileZero()) uses + * pg_pwritev_with_retry() to avoid multiple writes or needing a + * zeroed buffer for the whole length of the extension. + */ + ret = FileZero(v->mdfd_vfd, + seekpos, (off_t) BLCKSZ * numblocks, + WAIT_EVENT_DATA_FILE_EXTEND); + if (ret < 0) + ereport(ERROR, + errcode_for_file_access(), + errmsg("could not extend file \"%s\": %m", + FilePathName(v->mdfd_vfd)), + errhint("Check free disk space.")); + } + + if (!skipFsync && !SmgrIsTemp(reln)) + register_dirty_segment(reln, forknum, v); + + Assert(_mdnblocks(reln, forknum, v) <= ((BlockNumber) RELSEG_SIZE)); + + remblocks -= numblocks; + curblocknum += numblocks; + } +} + +/* + * mdwriteback() -- Tell the kernel to write pages back to storage. + * + * This accepts a range of blocks because flushing several pages at once is + * considerably more efficient than doing so individually. + */ +void +mdwriteback(SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, BlockNumber nblocks) +{ + Assert((io_direct_flags & IO_DIRECT_DATA) == 0); + + /* + * Issue flush requests in as few requests as possible; have to split at + * segment boundaries though, since those are actually separate files. + */ + while (nblocks > 0) + { + BlockNumber nflush = nblocks; + off_t seekpos; + MdfdVec *v; + int segnum_start, + segnum_end; + + v = _mdfd_getseg(reln, forknum, blocknum, true /* not used */ , + EXTENSION_DONT_OPEN); + + /* + * We might be flushing buffers of already removed relations, that's + * ok, just ignore that case. If the segment file wasn't open already + * (ie from a recent mdwrite()), then we don't want to re-open it, to + * avoid a race with PROCSIGNAL_BARRIER_SMGRRELEASE that might leave + * us with a descriptor to a file that is about to be unlinked. + */ + if (!v) + return; + + /* compute offset inside the current segment */ + segnum_start = blocknum / RELSEG_SIZE; + + /* compute number of desired writes within the current segment */ + segnum_end = (blocknum + nblocks - 1) / RELSEG_SIZE; + if (segnum_start != segnum_end) + nflush = RELSEG_SIZE - (blocknum % ((BlockNumber) RELSEG_SIZE)); + + Assert(nflush >= 1); + Assert(nflush <= nblocks); + + seekpos = (off_t) BLCKSZ * (blocknum % ((BlockNumber) RELSEG_SIZE)); + + FileWriteback(v->mdfd_vfd, seekpos, (off_t) BLCKSZ * nflush, WAIT_EVENT_DATA_FILE_FLUSH); + + nblocks -= nflush; + blocknum += nflush; + } +} + +/* + * mdnblocks() -- Get the number of blocks stored in a relation. * - * Important side effect: all active segments of the relation are opened - * and added to the md_seg_fds array. If this routine has not been - * called, then only segments up to the last one actually touched - * are present in the array. + * Important side effect: all active segments of the relation are opened + * and added to the md_seg_fds array. If this routine has not been + * called, then only segments up to the last one actually touched + * are present in the array. */ BlockNumber mdnblocks(SMgrRelation reln, ForkNumber forknum) @@ -986,7 +979,7 @@ mdnblocks(SMgrRelation reln, ForkNumber forknum) } /* - * mdtruncate() -- Truncate relation to specified number of blocks. + * mdtruncate() -- Truncate relation to specified number of blocks. */ void mdtruncate(SMgrRelation reln, ForkNumber forknum, BlockNumber nblocks) @@ -1080,7 +1073,7 @@ mdtruncate(SMgrRelation reln, ForkNumber forknum, BlockNumber nblocks) } /* - * mdimmedsync() -- Immediately sync a relation to stable storage. + * mdimmedsync() -- Immediately sync a relation to stable storage. * * Note that only writes already issued are synced; this routine knows * nothing of dirty buffers that may exist inside the buffer manager. We diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c index 70d0d570b1..d983a30475 100644 --- a/src/backend/storage/smgr/smgr.c +++ b/src/backend/storage/smgr/smgr.c @@ -30,7 +30,9 @@ /* * This struct of function pointers defines the API between smgr.c and - * any individual storage manager module. Note that smgr subfunctions are + * any individual storage manager module. + * + * Note that smgr subfunctions are * generally expected to report problems via elog(ERROR). An exception is * that smgr_unlink should use elog(WARNING), rather than erroring out, * because we normally unlink relations during post-commit/abort cleanup, @@ -49,16 +51,16 @@ typedef struct f_smgr bool (*smgr_exists) (SMgrRelation reln, ForkNumber forknum); void (*smgr_unlink) (RelFileLocatorBackend rlocator, ForkNumber forknum, bool isRedo); - void (*smgr_extend) (SMgrRelation reln, ForkNumber forknum, - BlockNumber blocknum, const void *buffer, bool skipFsync); - void (*smgr_zeroextend) (SMgrRelation reln, ForkNumber forknum, - BlockNumber blocknum, int nblocks, bool skipFsync); bool (*smgr_prefetch) (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum); void (*smgr_read) (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, void *buffer); void (*smgr_write) (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, const void *buffer, bool skipFsync); + void (*smgr_extend) (SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, const void *buffer, bool skipFsync); + void (*smgr_zeroextend) (SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, int nblocks, bool skipFsync); void (*smgr_writeback) (SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, BlockNumber nblocks); BlockNumber (*smgr_nblocks) (SMgrRelation reln, ForkNumber forknum); @@ -104,8 +106,7 @@ static void smgrshutdown(int code, Datum arg); /* - * smgrinit(), smgrshutdown() -- Initialize or shut down storage - * managers. + * smgrinit() -- Initialize storage managers. * * Note: smgrinit is called during backend startup (normal or standalone * case), *not* during postmaster start. Therefore, any resources created @@ -142,9 +143,11 @@ smgrshutdown(int code, Datum arg) } /* - * smgropen() -- Return an SMgrRelation object, creating it if need be. + * smgropen() -- Return an SMgrRelation object, creating it if need be. + * + * "backend" is for temporary tables, otherwise InvalidBackendId. * - * This does not attempt to actually open the underlying file. + * This does not attempt to actually open the underlying file. */ SMgrRelation smgropen(RelFileLocator rlocator, BackendId backend) @@ -245,7 +248,7 @@ smgrclearowner(SMgrRelation *owner, SMgrRelation reln) } /* - * smgrexists() -- Does the underlying file for a fork exist? + * smgrexists() -- Does the underlying file for a fork exist? */ bool smgrexists(SMgrRelation reln, ForkNumber forknum) @@ -254,7 +257,7 @@ smgrexists(SMgrRelation reln, ForkNumber forknum) } /* - * smgrclose() -- Close and delete an SMgrRelation object. + * smgrclose() -- Close and delete an SMgrRelation object. */ void smgrclose(SMgrRelation reln) @@ -284,9 +287,9 @@ smgrclose(SMgrRelation reln) } /* - * smgrrelease() -- Release all resources used by this object. + * smgrrelease() -- Release all resources used by this object. * - * The object remains valid. + * The object remains valid. */ void smgrrelease(SMgrRelation reln) @@ -299,9 +302,9 @@ smgrrelease(SMgrRelation reln) } /* - * smgrreleaseall() -- Release resources used by all objects. + * smgrreleaseall() -- Release resources used by all objects. * - * This is called for PROCSIGNAL_BARRIER_SMGRRELEASE. + * This is called for PROCSIGNAL_BARRIER_SMGRRELEASE. */ void smgrreleaseall(void) @@ -320,7 +323,7 @@ smgrreleaseall(void) } /* - * smgrcloseall() -- Close all existing SMgrRelation objects. + * smgrcloseall() -- Close all existing SMgrRelation objects. */ void smgrcloseall(void) @@ -339,8 +342,8 @@ smgrcloseall(void) } /* - * smgrcloserellocator() -- Close SMgrRelation object for given RelFileLocator, - * if one exists. + * smgrcloserellocator() -- Close SMgrRelation object for given + * RelFileLocator, if one exists. * * This has the same effects as smgrclose(smgropen(rlocator)), but it avoids * uselessly creating a hashtable entry only to drop it again when no @@ -363,11 +366,13 @@ smgrcloserellocator(RelFileLocatorBackend rlocator) } /* - * smgrcreate() -- Create a new relation. + * smgrcreate() -- Create a new relation. * - * Given an already-created (but presumably unused) SMgrRelation, - * cause the underlying disk file or other storage for the fork - * to be created. + * Given an already-created (but presumably unused) SMgrRelation, cause the + * underlying disk file or other storage for the fork to be created. + * + * isRedo is true during recovery. In that case, the underlying storage may + * already exist. */ void smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo) @@ -376,13 +381,13 @@ smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo) } /* - * smgrdosyncall() -- Immediately sync all forks of all given relations + * smgrdosyncall() -- Immediately sync all forks of all given relations * - * All forks of all given relations are synced out to the store. + * All forks of all given relations are synced out to the store. * - * This is equivalent to FlushRelationBuffers() for each smgr relation, - * then calling smgrimmedsync() for all forks of each relation, but it's - * significantly quicker so should be preferred when possible. + * This is equivalent to FlushRelationBuffers() for each smgr relation, then + * calling smgrimmedsync() for all forks of each relation, but it's + * significantly quicker so should be preferred when possible. */ void smgrdosyncall(SMgrRelation *rels, int nrels) @@ -411,14 +416,13 @@ smgrdosyncall(SMgrRelation *rels, int nrels) } /* - * smgrdounlinkall() -- Immediately unlink all forks of all given relations + * smgrdounlinkall() -- Immediately unlink all forks of all given relations * - * All forks of all given relations are removed from the store. This - * should not be used during transactional operations, since it can't be - * undone. + * All forks of all given relations are removed from the store. This should + * not be used during transactional operations, since it can't be undone. * - * If isRedo is true, it is okay for the underlying file(s) to be gone - * already. + * If isRedo is true, it is okay for the underlying file(s) to be gone + * already. */ void smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo) @@ -483,15 +487,64 @@ smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo) pfree(rlocators); } +/* + * smgrprefetch() -- Initiate asynchronous read of the specified block of a relation. + * + * In recovery only, this can return false to indicate that a file doesn't + * exist (presumably it has been dropped by a later WAL record). + */ +bool +smgrprefetch(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum) +{ + return smgrsw[reln->smgr_which].smgr_prefetch(reln, forknum, blocknum); +} + +/* + * smgrread() -- read a particular block from a relation into the supplied + * buffer. + * + * This routine is called from the buffer manager in order to instantiate + * pages in the shared buffer cache. All storage managers return pages in the + * format that POSTGRES expects. + */ +void +smgrread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, + void *buffer) +{ + smgrsw[reln->smgr_which].smgr_read(reln, forknum, blocknum, buffer); +} + +/* + * smgrwrite() -- Write the supplied buffer out. + * + * This is to be used only for updating already-existing blocks of a relation + * (ie, those before the current EOF). To extend a relation, use + * smgrextend(). + * + * This is not a synchronous write -- the block is not necessarily on disk at + * return, only dumped out to the kernel. However, provisions will be made to + * fsync the write before the next checkpoint. + * + * skipFsync indicates that the caller will make other provisions to fsync the + * relation, so we needn't bother. Temporary relations also do not require + * fsync. + */ +void +smgrwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, + const void *buffer, bool skipFsync) +{ + smgrsw[reln->smgr_which].smgr_write(reln, forknum, blocknum, + buffer, skipFsync); +} /* - * smgrextend() -- Add a new block to a file. + * smgrextend() -- Add a new block to a file. * - * The semantics are nearly the same as smgrwrite(): write at the - * specified position. However, this is to be used for the case of - * extending a relation (i.e., blocknum is at or beyond the current - * EOF). Note that we assume writing a block beyond current EOF - * causes intervening file space to become filled with zeroes. + * The semantics are nearly the same as smgrwrite(): write at the specified + * position. However, this is to be used for the case of extending a relation + * (i.e., blocknum is at or beyond the current EOF). Writing a block beyond + * current EOF must cause the intervening file space to become filled with + * zeroes. */ void smgrextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, @@ -512,11 +565,13 @@ smgrextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, } /* - * smgrzeroextend() -- Add new zeroed out blocks to a file. + * smgrzeroextend() -- Add new zeroed out blocks to a file. + * + * Extend the relation by the given number of blocks, which will be filled + * with zeroes. This is similar to smgrextend() but only extends and does not + * write a buffer of data. * - * Similar to smgrextend(), except the relation can be extended by - * multiple blocks at once and the added blocks will be filled with - * zeroes. + * FIXME: why both blocknum and nblocks */ void smgrzeroextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, @@ -537,60 +592,8 @@ smgrzeroextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, } /* - * smgrprefetch() -- Initiate asynchronous read of the specified block of a relation. - * - * In recovery only, this can return false to indicate that a file - * doesn't exist (presumably it has been dropped by a later WAL - * record). - */ -bool -smgrprefetch(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum) -{ - return smgrsw[reln->smgr_which].smgr_prefetch(reln, forknum, blocknum); -} - -/* - * smgrread() -- read a particular block from a relation into the supplied - * buffer. - * - * This routine is called from the buffer manager in order to - * instantiate pages in the shared buffer cache. All storage managers - * return pages in the format that POSTGRES expects. - */ -void -smgrread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, - void *buffer) -{ - smgrsw[reln->smgr_which].smgr_read(reln, forknum, blocknum, buffer); -} - -/* - * smgrwrite() -- Write the supplied buffer out. - * - * This is to be used only for updating already-existing blocks of a - * relation (ie, those before the current EOF). To extend a relation, - * use smgrextend(). - * - * This is not a synchronous write -- the block is not necessarily - * on disk at return, only dumped out to the kernel. However, - * provisions will be made to fsync the write before the next checkpoint. - * - * skipFsync indicates that the caller will make other provisions to - * fsync the relation, so we needn't bother. Temporary relations also - * do not require fsync. - */ -void -smgrwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, - const void *buffer, bool skipFsync) -{ - smgrsw[reln->smgr_which].smgr_write(reln, forknum, blocknum, - buffer, skipFsync); -} - - -/* - * smgrwriteback() -- Trigger kernel writeback for the supplied range of - * blocks. + * smgrwriteback() -- Trigger kernel writeback for the supplied range of + * blocks. */ void smgrwriteback(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, @@ -601,8 +604,8 @@ smgrwriteback(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, } /* - * smgrnblocks() -- Calculate the number of blocks in the - * supplied relation. + * smgrnblocks() -- Calculate the number of blocks in the + * supplied relation. */ BlockNumber smgrnblocks(SMgrRelation reln, ForkNumber forknum) @@ -622,8 +625,8 @@ smgrnblocks(SMgrRelation reln, ForkNumber forknum) } /* - * smgrnblocks_cached() -- Get the cached number of blocks in the supplied - * relation. + * smgrnblocks_cached() -- Get the cached number of blocks in the supplied + * relation. * * Returns an InvalidBlockNumber when not in recovery and when the relation * fork size is not cached. @@ -642,8 +645,8 @@ smgrnblocks_cached(SMgrRelation reln, ForkNumber forknum) } /* - * smgrtruncate() -- Truncate the given forks of supplied relation to - * each specified numbers of blocks + * smgrtruncate() -- Truncate the given forks of supplied relation to + * each specified numbers of blocks * * The truncation is done immediately, so this can't be rolled back. * @@ -694,27 +697,27 @@ smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks, BlockNumber *nb } /* - * smgrimmedsync() -- Force the specified relation to stable storage. - * - * Synchronously force all previous writes to the specified relation - * down to disk. - * - * This is useful for building completely new relations (eg, new - * indexes). Instead of incrementally WAL-logging the index build - * steps, we can just write completed index pages to disk with smgrwrite - * or smgrextend, and then fsync the completed index file before - * committing the transaction. (This is sufficient for purposes of - * crash recovery, since it effectively duplicates forcing a checkpoint - * for the completed index. But it is *not* sufficient if one wishes - * to use the WAL log for PITR or replication purposes: in that case - * we have to make WAL entries as well.) - * - * The preceding writes should specify skipFsync = true to avoid - * duplicative fsyncs. - * - * Note that you need to do FlushRelationBuffers() first if there is - * any possibility that there are dirty buffers for the relation; - * otherwise the sync is not very meaningful. + * smgrimmedsync() -- Force the specified relation to stable storage. + * + * Synchronously force all previous writes to the specified relation + * down to disk. + * + * This is useful for building completely new relations (eg, new + * indexes). Instead of incrementally WAL-logging the index build + * steps, we can just write completed index pages to disk with smgrwrite + * or smgrextend, and then fsync the completed index file before + * committing the transaction. (This is sufficient for purposes of + * crash recovery, since it effectively duplicates forcing a checkpoint + * for the completed index. But it is *not* sufficient if one wishes + * to use the WAL log for PITR or replication purposes: in that case + * we have to make WAL entries as well.) + * + * The preceding writes should specify skipFsync = true to avoid + * duplicative fsyncs. + * + * Note that you need to do FlushRelationBuffers() first if there is + * any possibility that there are dirty buffers for the relation; + * otherwise the sync is not very meaningful. */ void smgrimmedsync(SMgrRelation reln, ForkNumber forknum) diff --git a/src/include/storage/md.h b/src/include/storage/md.h index 941879ee6a..8af34e4155 100644 --- a/src/include/storage/md.h +++ b/src/include/storage/md.h @@ -26,16 +26,16 @@ extern void mdclose(SMgrRelation reln, ForkNumber forknum); extern void mdcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo); extern bool mdexists(SMgrRelation reln, ForkNumber forknum); extern void mdunlink(RelFileLocatorBackend rlocator, ForkNumber forknum, bool isRedo); -extern void mdextend(SMgrRelation reln, ForkNumber forknum, - BlockNumber blocknum, const void *buffer, bool skipFsync); -extern void mdzeroextend(SMgrRelation reln, ForkNumber forknum, - BlockNumber blocknum, int nblocks, bool skipFsync); extern bool mdprefetch(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum); extern void mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, void *buffer); extern void mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, const void *buffer, bool skipFsync); +extern void mdextend(SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, const void *buffer, bool skipFsync); +extern void mdzeroextend(SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, int nblocks, bool skipFsync); extern void mdwriteback(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, BlockNumber nblocks); extern BlockNumber mdnblocks(SMgrRelation reln, ForkNumber forknum); diff --git a/src/include/storage/smgr.h b/src/include/storage/smgr.h index a9a179aaba..896512f1bc 100644 --- a/src/include/storage/smgr.h +++ b/src/include/storage/smgr.h @@ -90,16 +90,16 @@ extern void smgrreleaseall(void); extern void smgrcreate(SMgrRelation reln, ForkNumber forknum, bool isRedo); extern void smgrdosyncall(SMgrRelation *rels, int nrels); extern void smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo); -extern void smgrextend(SMgrRelation reln, ForkNumber forknum, - BlockNumber blocknum, const void *buffer, bool skipFsync); -extern void smgrzeroextend(SMgrRelation reln, ForkNumber forknum, - BlockNumber blocknum, int nblocks, bool skipFsync); extern bool smgrprefetch(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum); extern void smgrread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, void *buffer); extern void smgrwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, const void *buffer, bool skipFsync); +extern void smgrextend(SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, const void *buffer, bool skipFsync); +extern void smgrzeroextend(SMgrRelation reln, ForkNumber forknum, + BlockNumber blocknum, int nblocks, bool skipFsync); extern void smgrwriteback(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum, BlockNumber nblocks); extern BlockNumber smgrnblocks(SMgrRelation reln, ForkNumber forknum); -- 2.40.1

view thread (11+ messages)  latest in thread

Message-ID: <c9e36e90-3674-e256-4bec-9971cacd4ca2@enterprisedb.com>
Permalink:  ../c9e36e90-3674-e256-4bec-9971cacd4ca2@enterprisedb.com/
Also on:    postgresql.org/message-id/c9e36e90-3674-e256-4bec-9971cacd4ca2@enterprisedb.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: peter.eisentraut@enterprisedb.com, andres@anarazel.de
  Subject: Re: smgrzeroextend clarification
  In-Reply-To: <c9e36e90-3674-e256-4bec-9971cacd4ca2@enterprisedb.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox