BufmgrCommit no-op since 2008, remaining uses?
Hi,
Whilst reading up on the transaction commit code, I noticed the following lines:
/* Tell bufmgr and smgr to prepare for commit */
BufmgrCommit();
BufmgrCommit does exactly nothing; it is an empty function and has
been since commit 33960006 in late 2008 when it stopped calling
smgrcommit().
All two usages of the function (in our code base) seem to be in
xact.c. Are we maintaining it for potential future use, or can the
function be removed?
Kind regards,
Matthias van de Meent
Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
BufmgrCommit does exactly nothing; it is an empty function and has
been since commit 33960006 in late 2008 when it stopped calling
smgrcommit().
All two usages of the function (in our code base) seem to be in
xact.c. Are we maintaining it for potential future use, or can the
function be removed?
Seems reasonable. Even if bufmgr grew a new need to be called
during commit, it would quite possibly need to be called from
a different spot; so I doubt that the function is useful even
as a placeholder.
regards, tom lane
On Wed, 29 Mar 2023 at 14:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Matthias van de Meent <boekewurm+postgres@gmail.com> writes:
BufmgrCommit does exactly nothing; it is an empty function and has
been since commit 33960006 in late 2008 when it stopped calling
smgrcommit().
All two usages of the function (in our code base) seem to be in
xact.c. Are we maintaining it for potential future use, or can the
function be removed?Seems reasonable. Even if bufmgr grew a new need to be called
during commit, it would quite possibly need to be called from
a different spot; so I doubt that the function is useful even
as a placeholder.
Then, the attached trivial patch removes the function and all
references I could find.
Kind regards,
Matthias van de Meent
Attachments:
0001-Cleanup-Drop-BufmgrCommit.patchapplication/octet-stream; name=0001-Cleanup-Drop-BufmgrCommit.patchDownload
From 859679e72b10a45fd683bc45cd225482e60f6b6f Mon Sep 17 00:00:00 2001
From: Matthias van de Meent <boekewurm+postgres@gmail.com>
Date: Wed, 29 Mar 2023 14:45:58 +0200
Subject: [PATCH] Cleanup: Drop BufmgrCommit
The function has been a no-op since commit 33960006, and any
future equivalent may not be called from the same location.
---
src/backend/access/transam/xact.c | 5 -----
src/backend/storage/buffer/bufmgr.c | 10 ----------
src/include/storage/bufmgr.h | 1 -
3 files changed, 16 deletions(-)
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index b876401260..01b1e0fb8c 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1376,8 +1376,6 @@ RecordTransactionCommit(void)
/*
* Begin commit critical section and insert the commit XLOG record.
*/
- /* Tell bufmgr and smgr to prepare for commit */
- BufmgrCommit();
/*
* Mark ourselves as within our "commit critical section". This
@@ -2536,9 +2534,6 @@ PrepareTransaction(void)
prepared_at = GetCurrentTimestamp();
- /* Tell bufmgr and smgr to prepare for commit */
- BufmgrCommit();
-
/*
* Reserve the GID for this transaction. This could fail if the requested
* GID is invalid or already in use.
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 98904a7c05..7f2d11c8cc 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -2789,16 +2789,6 @@ CheckPointBuffers(int flags)
BufferSync(flags);
}
-
-/*
- * Do whatever is needed to prepare for commit at the bufmgr and smgr levels
- */
-void
-BufmgrCommit(void)
-{
- /* Nothing to do in bufmgr anymore... */
-}
-
/*
* BufferGetBlockNumber
* Returns the block number associated with a buffer.
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
index b8a18b8081..73762cb1ec 100644
--- a/src/include/storage/bufmgr.h
+++ b/src/include/storage/bufmgr.h
@@ -182,7 +182,6 @@ extern bool HoldingBufferPinThatDelaysRecovery(void);
extern void AbortBufferIO(void);
-extern void BufmgrCommit(void);
extern bool BgBufferSync(struct WritebackContext *wb_context);
extern void TestForOldSnapshot_impl(Snapshot snapshot, Relation relation);
--
2.39.0