Thread
-
[PATCH v35 10/21] Rename confusing function names
Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2024-10-25T04:02:22Z
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"