[PATCH v35 10/21] Rename confusing function names
Kyotaro Horiguchi <horikyota.ntt@gmail.com>
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
To:
Date: 2024-10-25T04:02:22Z
Lists: pgsql-hackers
This is the final step in a series of three commits to modify
pendingDeletes.
The pendingDeletes system now handles commit-time processing
only. Previously, its structures and functions managed both commit and
abort cases, so removing abort-time processing alone could cause
confusion. Therefore, update the function names to clarify their
commit-time purpose and remove unnecessary safeguards.
---
src/backend/access/transam/twophase.c | 2 +-
src/backend/access/transam/xact.c | 2 +-
src/backend/catalog/storage.c | 44 +++++++++------------------
src/backend/commands/tablecmds.c | 2 +-
src/include/catalog/storage.h | 4 +--
5 files changed, 19 insertions(+), 35 deletions(-)
diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index d3dac67d8cd..ef94aea4ee9 100644
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -1113,7 +1113,7 @@ StartPrepare(GlobalTransaction gxact)
hdr.prepared_at = gxact->prepared_at;
hdr.owner = gxact->owner;
hdr.nsubxacts = xactGetCommittedChildren(&children);
- hdr.ncommitrels = smgrGetPendingDeletes(true, &commitrels);
+ hdr.ncommitrels = smgrGetCommitPendingDeletes(&commitrels);
hdr.ncommitstats =
pgstat_get_transactional_drops(true, &commitstats);
hdr.nabortstats =
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 1a691c7a30e..33e99bd31ae 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1340,7 +1340,7 @@ RecordTransactionCommit(void)
LogLogicalInvalidations();
/* Get data needed for commit record */
- nrels = smgrGetPendingDeletes(true, &rels);
+ nrels = smgrGetCommitPendingDeletes(&rels);
nchildren = xactGetCommittedChildren(&children);
ndroppedstats = pgstat_get_transactional_drops(true, &droppedstats);
if (XLogStandbyInfoActive())
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
index db9cdad25f8..043afa47ced 100644
--- a/src/backend/catalog/storage.c
+++ b/src/backend/catalog/storage.c
@@ -252,45 +252,31 @@ RelationDropStorage(Relation rel)
}
/*
- * RelationPreserveStorage
+ * RelationPreserveStorageOnCommit
* Mark a relation as not to be deleted after all.
*
- * We need this function because relation mapping changes are committed
- * separately from commit of the whole transaction, so it's still possible
- * for the transaction to abort after the mapping update is done.
- * When a new physical relation is installed in the map, it would be
- * scheduled for delete-on-abort, so we'd delete it, and be in trouble.
- * The relation mapper fixes this by telling us to not delete such relations
- * after all as part of its commit.
+ * This function cancels registered delete-on-commit actions for storage files,
+ * allowing reuse of an existing index build during ALTER TABLE.
*
- * We also use this to reuse an old build of an index during ALTER TABLE, this
- * time removing the delete-at-commit entry.
+ * Historical note:
+ * The only caller in abort paths was write_relmapper_file(), intended to
+ * preserve committed storage files for mapped relations if outer
+ * transactions aborted. However, this case has not occurred in over a decade
+ * and is unlikely to be needed in the future. Maintaining the ability for
+ * subtransaction-committed storage files to persist after a top-level
+ * transaction aborts has added unnecessary complexity and inefficiency to
+ * the UNDO log system. Therefore, this feature has been removed, and the
+ * function has been renamed.
*
* No-op if the relation is not among those scheduled for deletion.
*/
void
-RelationPreserveStorage(RelFileLocator rlocator, bool atCommit)
+RelationPreserveStorageOnCommit(RelFileLocator rlocator)
{
PendingRelDelete *pending;
PendingRelDelete *prev;
PendingRelDelete *next;
- /*
- * There is no caller that passes false for atCommit.
- *
- * The only caller that used to pass false for atCommit was
- * write_relmapper_file(), which intended to preserve committed storage
- * files for mapped relations if outer transactions aborted. However, this
- * has not occurred for more than ten years, and it is unlikely to be
- * needed in the future. The code to let storage files committed in
- * subtransactions survive after the top transaction aborts makes the UNDO
- * log system overly complex and inefficient. Therefore, this feature has
- * been removed. The function signature is left unchanged to make this
- * change less invasive and to prevent the function from being mistakenly
- * called during transaction aborts.
- */
- Assert (atCommit);
-
prev = NULL;
for (pending = pendingDeletes; pending != NULL; pending = next)
{
@@ -883,15 +869,13 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker)
* by upper-level transactions.
*/
int
-smgrGetPendingDeletes(bool forCommit, RelFileLocator **ptr)
+smgrGetCommitPendingDeletes(RelFileLocator **ptr)
{
int nestLevel = GetCurrentTransactionNestLevel();
int nrels;
RelFileLocator *rptr;
PendingRelDelete *pending;
- Assert(forCommit);
-
nrels = 0;
for (pending = pendingDeletes; pending != NULL; pending = pending->next)
{
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 56c9d61aa21..251aea55d24 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -9197,7 +9197,7 @@ ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
irel->rd_createSubid = stmt->oldCreateSubid;
irel->rd_firstRelfilelocatorSubid = stmt->oldFirstRelfilelocatorSubid;
- RelationPreserveStorage(irel->rd_locator, true);
+ RelationPreserveStorageOnCommit(irel->rd_locator);
index_close(irel, NoLock);
}
diff --git a/src/include/catalog/storage.h b/src/include/catalog/storage.h
index 3451d6ac80c..19b02d84a5f 100644
--- a/src/include/catalog/storage.h
+++ b/src/include/catalog/storage.h
@@ -28,7 +28,7 @@ extern SMgrRelation RelationCreateStorage(RelFileLocator rlocator,
extern void RelationCreateFork(SMgrRelation srel, ForkNumber forkNum,
bool wal_log, bool undo_log);
extern void RelationDropStorage(Relation rel);
-extern void RelationPreserveStorage(RelFileLocator rlocator, bool atCommit);
+extern void RelationPreserveStorageOnCommit(RelFileLocator rlocator);
extern void RelationPreTruncate(Relation rel);
extern void RelationTruncate(Relation rel, BlockNumber nblocks);
extern void RelationCopyStorage(SMgrRelation src, SMgrRelation dst,
@@ -44,7 +44,7 @@ extern void RestorePendingSyncs(char *startAddress);
*/
extern void smgrDoPendingDeletes(bool isCommit);
extern void smgrDoPendingSyncs(bool isCommit, bool isParallelWorker);
-extern int smgrGetPendingDeletes(bool forCommit, RelFileLocator **ptr);
+extern int smgrGetCommitPendingDeletes(RelFileLocator **ptr);
extern void AtSubCommit_smgr(void);
extern void AtSubAbort_smgr(void);
extern void PostPrepare_smgr(void);
--
2.43.5
----Next_Part(Thu_Oct_31_17_01_30_2024_620)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="v35-0011-new-indexam-bit-for-unlogged-storage-compatibili.patch"