At Fri, 08 Jul 2022 11:52:45 +0900 (JST), Kyotaro Horiguchi
<horikyota....@gmail.com> wrote in
> > I wouldn't mind if we took "Locator" out of the name of that function
> > and just called it DropRelFileBuffers or DropRelationBuffers or
> > something. That would be shorter, and maybe more intuitive.
>
> Thanks. Will propose that.
I thought for a moment that "Relation" sounded better but that naming
is confusing in bufmgr.c, where functions take Relation and those take
RelFileLocator exist together. So the (second) attached introduces
"RelFile" to represent RelFileNode excluding RelFileLocator.
The function CreateAndCopyRelationData exists since before b0a55e4329
but renamed since it takes RelFileLocators.
While working on this, I found that the following coment is wrong.
* FlushRelationsAllBuffers
*
* This function flushes out of the buffer pool all the pages of
all
* forks of the specified smgr relations. It's equivalent to
calling
* FlushRelationBuffers once per fork per relation. The relations
are
* assumed not to use local buffers.
It is equivalent to calling FlushRelationBuffers "per relation". This
is attached as the first patch, which could be thought as a separate
patch.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
>From 83b0e0d65bb9ea47cb3aa0fade84b009f2d1b13b Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Fri, 8 Jul 2022 14:37:41 +0900
Subject: [PATCH v1 1/2] Fix a comment
FlushRelFileAllBuffers is equivalent to calling FlushRelationBuffers
not "per fork per relation", but "per fork".
---
src/backend/storage/buffer/bufmgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index e4de4b306c..e257ae23e4 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -3589,8 +3589,8 @@ FlushRelationBuffers(Relation rel)
*
* This function flushes out of the buffer pool all the pages of all
* forks of the specified smgr relations. It's equivalent to calling
- * FlushRelationBuffers once per fork per relation. The relations are
- * assumed not to use local buffers.
+ * FlushRelationBuffers once per relation. The relations are assumed not
+ * to use local buffers.
* --------------------------------------------------------------------
*/
void
--
2.31.1
>From 37e3019335cb0780c393a49e30142e1d9c9caf2a Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota....@gmail.com>
Date: Fri, 8 Jul 2022 13:20:31 +0900
Subject: [PATCH v1 2/2] Rename some *RelFileLocator* functions
A recent commit b0a55e4329 changed the term RelFileNode to
RelFileLocator. Accordingly the names of some functions were
changed. However, since RelFileLocator doesn't fully cover what
RelFileNode represented, some of the function names have gotten less
intuitive.
This commit (re)introduces the word RelFile, which means the file
pointed by a RelFileLocator, then renames the affected functions and a
function that b0a55e4329 did not touch in harmonization with the new
convention that the names of the functions that take RelFileLocator
use RelFile as object.
---
src/backend/commands/dbcommands.c | 2 +-
src/backend/storage/buffer/bufmgr.c | 73 +++++++++++++--------------
src/backend/storage/buffer/localbuf.c | 16 +++---
src/backend/storage/smgr/smgr.c | 6 +--
src/include/storage/buf_internals.h | 6 +--
src/include/storage/bufmgr.h | 18 +++----
6 files changed, 60 insertions(+), 61 deletions(-)
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 1901b434c5..e0bc26e16c 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -210,7 +210,7 @@ CreateDatabaseUsingWalLog(Oid src_dboid, Oid dst_dboid,
LockRelationId(&dstrelid, AccessShareLock);
/* Copy relation storage from source to the destination. */
- CreateAndCopyRelationData(srcrlocator, dstrlocator, relinfo->permanent);
+ CreateAndCopyRelFileData(srcrlocator, dstrlocator, relinfo->permanent);
/* Release the relation locks. */
UnlockRelationId(&srcrelid, AccessShareLock);
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index e257ae23e4..31397641a7 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -120,8 +120,8 @@ typedef struct CkptTsStatus
/*
* Type for array used to sort SMgrRelations
*
- * FlushRelationsAllBuffers shares the same comparator function with
- * DropRelFileLocatorsAllBuffers. Pointer to this struct and RelFileLocator must be
+ * FlushRelFilesAllBuffers shares the same comparator function with
+ * DropRelFilesAllBuffers. Pointer to this struct and RelFileLocator must be
* compatible.
*/
typedef struct SMgrSortArray
@@ -483,10 +483,10 @@ static BufferDesc *BufferAlloc(SMgrRelation smgr,
BufferAccessStrategy strategy,
bool *foundPtr);
static void FlushBuffer(BufferDesc *buf, SMgrRelation reln);
-static void FindAndDropRelFileLocatorBuffers(RelFileLocator rlocator,
- ForkNumber forkNum,
- BlockNumber nForkBlock,
- BlockNumber firstDelBlock);
+static void FindAndDropRelFileBuffers(RelFileLocator rlocator,
+ ForkNumber forkNum,
+ BlockNumber nForkBlock,
+ BlockNumber firstDelBlock);
static void RelationCopyStorageUsingBuffer(Relation src, Relation dst,
ForkNumber forkNum,
bool isunlogged);
@@ -3026,7 +3026,7 @@ BufferGetLSNAtomic(Buffer buffer)
}
/* ---------------------------------------------------------------------
- * DropRelFileLocatorBuffers
+ * DropRelFileBuffers
*
* This function removes from the buffer pool all the pages of the
* specified relation forks that have block numbers >= firstDelBlock.
@@ -3047,8 +3047,8 @@ BufferGetLSNAtomic(Buffer buffer)
* --------------------------------------------------------------------
*/
void
-DropRelFileLocatorBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
- int nforks, BlockNumber *firstDelBlock)
+DropRelFileBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
+ int nforks, BlockNumber *firstDelBlock)
{
int i;
int j;
@@ -3064,8 +3064,8 @@ DropRelFileLocatorBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
if (rlocator.backend == MyBackendId)
{
for (j = 0; j < nforks; j++)
- DropRelFileLocatorLocalBuffers(rlocator.locator, forkNum[j],
- firstDelBlock[j]);
+ DropRelFileLocalBuffers(rlocator.locator, forkNum[j],
+ firstDelBlock[j]);
}
return;
}
@@ -3115,8 +3115,8 @@ DropRelFileLocatorBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
nBlocksToInvalidate < BUF_DROP_FULL_SCAN_THRESHOLD)
{
for (j = 0; j < nforks; j++)
- FindAndDropRelFileLocatorBuffers(rlocator.locator, forkNum[j],
- nForkBlock[j], firstDelBlock[j]);
+ FindAndDropRelFileBuffers(rlocator.locator, forkNum[j],
+ nForkBlock[j], firstDelBlock[j]);
return;
}
@@ -3162,16 +3162,15 @@ DropRelFileLocatorBuffers(SMgrRelation smgr_reln, ForkNumber *forkNum,
}
/* ---------------------------------------------------------------------
- * DropRelFileLocatorsAllBuffers
+ * DropRelFilesAllBuffers
*
* This function removes from the buffer pool all the pages of all
* forks of the specified relations. It's equivalent to calling
- * DropRelFileLocatorBuffers once per fork per relation with
- * firstDelBlock = 0.
- * --------------------------------------------------------------------
+ * DropRelFileBuffers once per fork per relation with firstDelBlock = 0.
+ * --------------------------------------------------------------------
*/
void
-DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
+DropRelFilesAllBuffers(SMgrRelation *smgr_reln, int nlocators)
{
int i;
int j;
@@ -3194,7 +3193,7 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
if (RelFileLocatorBackendIsTemp(smgr_reln[i]->smgr_rlocator))
{
if (smgr_reln[i]->smgr_rlocator.backend == MyBackendId)
- DropRelFileLocatorAllLocalBuffers(smgr_reln[i]->smgr_rlocator.locator);
+ DropRelFileAllLocalBuffers(smgr_reln[i]->smgr_rlocator.locator);
}
else
rels[n++] = smgr_reln[i];
@@ -3219,7 +3218,7 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
/*
* We can avoid scanning the entire buffer pool if we know the exact size
- * of each of the given relation forks. See DropRelFileLocatorBuffers.
+ * of each of the given relation forks. See DropRelFileBuffers.
*/
for (i = 0; i < n && cached; i++)
{
@@ -3257,8 +3256,8 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
continue;
/* drop all the buffers for a particular relation fork */
- FindAndDropRelFileLocatorBuffers(rels[i]->smgr_rlocator.locator,
- j, block[i][j], 0);
+ FindAndDropRelFileBuffers(rels[i]->smgr_rlocator.locator,
+ j, block[i][j], 0);
}
}
@@ -3291,7 +3290,7 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
uint32 buf_state;
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelFileBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
@@ -3331,7 +3330,7 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
}
/* ---------------------------------------------------------------------
- * FindAndDropRelFileLocatorBuffers
+ * FindAndDropRelFileBuffers
*
* This function performs look up in BufMapping table and removes from the
* buffer pool all the pages of the specified relation fork that has block
@@ -3340,9 +3339,9 @@ DropRelFileLocatorsAllBuffers(SMgrRelation *smgr_reln, int nlocators)
* --------------------------------------------------------------------
*/
static void
-FindAndDropRelFileLocatorBuffers(RelFileLocator rlocator, ForkNumber forkNum,
- BlockNumber nForkBlock,
- BlockNumber firstDelBlock)
+FindAndDropRelFileBuffers(RelFileLocator rlocator, ForkNumber forkNum,
+ BlockNumber nForkBlock,
+ BlockNumber firstDelBlock)
{
BlockNumber curBlock;
@@ -3397,7 +3396,7 @@ FindAndDropRelFileLocatorBuffers(RelFileLocator rlocator, ForkNumber forkNum,
* bothering to write them out first. This is used when we destroy a
* database, to avoid trying to flush data to disk when the directory
* tree no longer exists. Implementation is pretty similar to
- * DropRelFileLocatorBuffers() which is for destroying just one relation.
+ * DropRelFileBuffers() which is for destroying just one relation.
* --------------------------------------------------------------------
*/
void
@@ -3416,7 +3415,7 @@ DropDatabaseBuffers(Oid dbid)
uint32 buf_state;
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelFileBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
if (bufHdr->tag.rlocator.dbOid != dbid)
@@ -3561,7 +3560,7 @@ FlushRelationBuffers(Relation rel)
bufHdr = GetBufferDescriptor(i);
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelFileBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
if (!RelFileLocatorEquals(bufHdr->tag.rlocator, rel->rd_locator))
@@ -3585,7 +3584,7 @@ FlushRelationBuffers(Relation rel)
}
/* ---------------------------------------------------------------------
- * FlushRelationsAllBuffers
+ * FlushRelFilesAllBuffers
*
* This function flushes out of the buffer pool all the pages of all
* forks of the specified smgr relations. It's equivalent to calling
@@ -3594,7 +3593,7 @@ FlushRelationBuffers(Relation rel)
* --------------------------------------------------------------------
*/
void
-FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
+FlushRelFilesAllBuffers(SMgrRelation *smgrs, int nrels)
{
int i;
SMgrSortArray *srels;
@@ -3616,7 +3615,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
/*
* Save the bsearch overhead for low number of relations to sync. See
- * DropRelFileLocatorsAllBuffers for details.
+ * DropRelFilesAllBuffers for details.
*/
use_bsearch = nrels > RELS_BSEARCH_THRESHOLD;
@@ -3634,7 +3633,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
uint32 buf_state;
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelFileBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
@@ -3775,8 +3774,8 @@ RelationCopyStorageUsingBuffer(Relation src, Relation dst, ForkNumber forkNum,
* --------------------------------------------------------------------
*/
void
-CreateAndCopyRelationData(RelFileLocator src_rlocator,
- RelFileLocator dst_rlocator, bool permanent)
+CreateAndCopyRelFileData(RelFileLocator src_rlocator,
+ RelFileLocator dst_rlocator, bool permanent)
{
Relation src_rel;
Relation dst_rel;
@@ -3864,7 +3863,7 @@ FlushDatabaseBuffers(Oid dbid)
bufHdr = GetBufferDescriptor(i);
/*
- * As in DropRelFileLocatorBuffers, an unlocked precheck should be
+ * As in DropRelFileBuffers, an unlocked precheck should be
* safe and saves some cycles.
*/
if (bufHdr->tag.rlocator.dbOid != dbid)
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
index 41a08076b3..2f496767c9 100644
--- a/src/backend/storage/buffer/localbuf.c
+++ b/src/backend/storage/buffer/localbuf.c
@@ -312,7 +312,7 @@ MarkLocalBufferDirty(Buffer buffer)
}
/*
- * DropRelFileLocatorLocalBuffers
+ * DropRelFileLocalBuffers
* This function removes from the buffer pool all the pages of the
* specified relation that have block numbers >= firstDelBlock.
* (In particular, with firstDelBlock = 0, all pages are removed.)
@@ -320,11 +320,11 @@ MarkLocalBufferDirty(Buffer buffer)
* out first. Therefore, this is NOT rollback-able, and so should be
* used only with extreme caution!
*
- * See DropRelFileLocatorBuffers in bufmgr.c for more notes.
+ * See DropRelFileBuffers in bufmgr.c for more notes.
*/
void
-DropRelFileLocatorLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
- BlockNumber firstDelBlock)
+DropRelFileLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
+ BlockNumber firstDelBlock)
{
int i;
@@ -363,14 +363,14 @@ DropRelFileLocatorLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
}
/*
- * DropRelFileLocatorAllLocalBuffers
+ * DropRelFileAllLocalBuffers
* This function removes from the buffer pool all pages of all forks
* of the specified relation.
*
- * See DropRelFileLocatorsAllBuffers in bufmgr.c for more notes.
+ * See DropRelFilesAllBuffers in bufmgr.c for more notes.
*/
void
-DropRelFileLocatorAllLocalBuffers(RelFileLocator rlocator)
+DropRelFileAllLocalBuffers(RelFileLocator rlocator)
{
int i;
@@ -589,7 +589,7 @@ AtProcExit_LocalBuffers(void)
{
/*
* We shouldn't be holding any remaining pins; if we are, and assertions
- * aren't enabled, we'll fail later in DropRelFileLocatorBuffers while
+ * aren't enabled, we'll fail later in DropRelFileBuffers while
* trying to drop the temp rels.
*/
CheckForLocalBufferLeaks();
diff --git a/src/backend/storage/smgr/smgr.c b/src/backend/storage/smgr/smgr.c
index b21d8c3822..8dc15dda65 100644
--- a/src/backend/storage/smgr/smgr.c
+++ b/src/backend/storage/smgr/smgr.c
@@ -389,7 +389,7 @@ smgrdosyncall(SMgrRelation *rels, int nrels)
if (nrels == 0)
return;
- FlushRelationsAllBuffers(rels, nrels);
+ FlushRelFilesAllBuffers(rels, nrels);
/*
* Sync the physical file(s).
@@ -430,7 +430,7 @@ smgrdounlinkall(SMgrRelation *rels, int nrels, bool isRedo)
* Get rid of any remaining buffers for the relations. bufmgr will just
* drop them without bothering to write the contents.
*/
- DropRelFileLocatorsAllBuffers(rels, nrels);
+ DropRelFilesAllBuffers(rels, nrels);
/*
* create an array which contains all relations to be dropped, and close
@@ -631,7 +631,7 @@ smgrtruncate(SMgrRelation reln, ForkNumber *forknum, int nforks, BlockNumber *nb
* Get rid of any buffers for the about-to-be-deleted blocks. bufmgr will
* just drop them without bothering to write the contents.
*/
- DropRelFileLocatorBuffers(reln, forknum, nforks, nblocks);
+ DropRelFileBuffers(reln, forknum, nforks, nblocks);
/*
* Send a shared-inval message to force other backends to close any smgr
diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index aded5e8f7e..4b2150b980 100644
--- a/src/include/storage/buf_internals.h
+++ b/src/include/storage/buf_internals.h
@@ -337,9 +337,9 @@ extern PrefetchBufferResult PrefetchLocalBuffer(SMgrRelation smgr,
extern BufferDesc *LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum,
BlockNumber blockNum, bool *foundPtr);
extern void MarkLocalBufferDirty(Buffer buffer);
-extern void DropRelFileLocatorLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
- BlockNumber firstDelBlock);
-extern void DropRelFileLocatorAllLocalBuffers(RelFileLocator rlocator);
+extern void DropRelFileLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum,
+ BlockNumber firstDelBlock);
+extern void DropRelFileAllLocalBuffers(RelFileLocator rlocator);
extern void AtEOXact_LocalBuffers(bool isCommit);
#endif /* BUFMGR_INTERNALS_H */
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index 7bcfaac272..e6bc7201db 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -203,16 +203,16 @@ extern BlockNumber RelationGetNumberOfBlocksInFork(Relation relation,
ForkNumber forkNum);
extern void FlushOneBuffer(Buffer buffer);
extern void FlushRelationBuffers(Relation rel);
-extern void FlushRelationsAllBuffers(struct SMgrRelationData **smgrs, int nrels);
-extern void CreateAndCopyRelationData(RelFileLocator src_rlocator,
- RelFileLocator dst_rlocator,
- bool permanent);
+extern void FlushRelFilesAllBuffers(struct SMgrRelationData **smgrs, int nrels);
+extern void CreateAndCopyRelFileData(RelFileLocator src_rlocator,
+ RelFileLocator dst_rlocator,
+ bool permanent);
extern void FlushDatabaseBuffers(Oid dbid);
-extern void DropRelFileLocatorBuffers(struct SMgrRelationData *smgr_reln,
- ForkNumber *forkNum,
- int nforks, BlockNumber *firstDelBlock);
-extern void DropRelFileLocatorsAllBuffers(struct SMgrRelationData **smgr_reln,
- int nlocators);
+extern void DropRelFileBuffers(struct SMgrRelationData *smgr_reln,
+ ForkNumber *forkNum,
+ int nforks, BlockNumber *firstDelBlock);
+extern void DropRelFilesAllBuffers(struct SMgrRelationData **smgr_reln,
+ int nlocators);
extern void DropDatabaseBuffers(Oid dbid);
#define RelationGetNumberOfBlocks(reln) \
--
2.31.1
