pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Anton A. Melnikov <a.melnikov@postgrespro.ru>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: daniel@yesql.se
Cc: peter@eisentraut.org
Cc: tgl@sss.pgh.pa.us
Cc: robertmhaas@gmail.com
Cc: michael@paquier.xyz
Cc: pgsql-hackers@postgresql.org
Subject: Re: Use XLOG_CONTROL_FILE macro everywhere?
Date: Sat, 5 Apr 2025 01:32:57 +0900
Message-ID: <18d80eb9-1e7a-4ead-98fe-60efc988954b@oss.nttdata.com> (raw )
In-Reply-To: <2883bd25-f362-41ed-8d41-b6e0aac8fa27@postgrespro.ru >
References: <54B16BF2-6979-4BB0-BE39-118D32B1BC58@yesql.se >
<20240903.143756.562059416508617821.horikyota.ntt@gmail.com >
<fc33823e-dbdf-4f31-b9aa-bf7f758019b9@postgrespro.ru >
<20240904.170927.1051663922860738119.horikyota.ntt@gmail.com >
<a131bf48-6d62-4b67-87e4-ff2c069350b2@postgrespro.ru >
<359568e0-cffc-4f64-b84c-eee76ba229b8@postgrespro.ru >
<2883bd25-f362-41ed-8d41-b6e0aac8fa27@postgrespro.ru >
On 2025/03/28 13:09, Anton A. Melnikov wrote:
> Hi!
>
>> * Patch needs rebase by CFbot
>
> Rebased the patches onto the current master and marked them as v4.
I'm not sure there's clear consensus yet on the changes in the 0001 and
0002 patches, and it might not be worth rushing them in right before
the feature freeze. So for now, I reviewed and updated only the 0003 patch,
since there seems to be agreement on that one.
I've attached the updated version. It fixes a typo and replaces
the remaining hardcoded control file name with the XLOG_CONTROL_FILE macro.
How about committing the attached patch first? We can consider applying
the others later if consensus is reached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Attachments:
v5-0001-Use-XLOG_CONTROL_FILE-macro-consistently-for-cont.patch (text/plain, 10.4K) download
From 3c2891e042fec2d8251907782b02f5acdbd31ae6 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fujii@postgresql.org>
Date: Sat, 5 Apr 2025 00:20:32 +0900
Subject: [PATCH v5] Use XLOG_CONTROL_FILE macro consistently for control file
name.
The XLOG_CONTROL_FILE macro (defined in access/xlog_internal.h)
represents the control file name. While some parts of the codebase already
use this macro, others previously hardcoded the file name as a string.
This commit replaces those hardcoded strings with the macro,
ensuring consistent usage throughout the code. This makes future
maintenance easier and improves searchability, for example when
grepping for control file usage.
---
src/backend/backup/basebackup.c | 2 +-
src/backend/postmaster/postmaster.c | 3 ++-
src/bin/pg_combinebackup/pg_combinebackup.c | 5 +++--
src/bin/pg_controldata/pg_controldata.c | 2 +-
src/bin/pg_rewind/filemap.c | 3 ++-
src/bin/pg_rewind/pg_rewind.c | 8 ++++----
src/bin/pg_upgrade/controldata.c | 11 ++++++-----
src/bin/pg_verifybackup/astreamer_verify.c | 5 +++--
src/bin/pg_verifybackup/pg_verifybackup.c | 3 ++-
src/common/controldata_utils.c | 2 +-
10 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/src/backend/backup/basebackup.c b/src/backend/backup/basebackup.c
index 891637e3a44..f0f88838dc2 100644
--- a/src/backend/backup/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -1349,7 +1349,7 @@ sendDir(bbsink *sink, const char *path, int basepathlen, bool sizeonly,
snprintf(pathbuf, sizeof(pathbuf), "%s/%s", path, de->d_name);
/* Skip pg_control here to back up it last */
- if (strcmp(pathbuf, "./global/pg_control") == 0)
+ if (strcmp(pathbuf, "./" XLOG_CONTROL_FILE) == 0)
continue;
if (lstat(pathbuf, &statbuf) != 0)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 3fe45de5da0..17fed96fe20 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -90,6 +90,7 @@
#endif
#include "access/xlog.h"
+#include "access/xlog_internal.h"
#include "access/xlogrecovery.h"
#include "common/file_perm.h"
#include "common/pg_prng.h"
@@ -1516,7 +1517,7 @@ checkControlFile(void)
char path[MAXPGPATH];
FILE *fp;
- snprintf(path, sizeof(path), "%s/global/pg_control", DataDir);
+ snprintf(path, sizeof(path), "%s/%s", DataDir, XLOG_CONTROL_FILE);
fp = AllocateFile(path, PG_BINARY_R);
if (fp == NULL)
diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c
index d61bde42f49..4f8ba156336 100644
--- a/src/bin/pg_combinebackup/pg_combinebackup.c
+++ b/src/bin/pg_combinebackup/pg_combinebackup.c
@@ -24,6 +24,7 @@
#include <linux/fs.h>
#endif
+#include "access/xlog_internal.h"
#include "backup_label.h"
#include "common/checksum_helper.h"
#include "common/controldata_utils.h"
@@ -300,7 +301,7 @@ main(int argc, char *argv[])
{
char *controlpath;
- controlpath = psprintf("%s/%s", prior_backup_dirs[i], "global/pg_control");
+ controlpath = psprintf("%s/%s", prior_backup_dirs[i], XLOG_CONTROL_FILE);
pg_fatal("%s: manifest system identifier is %" PRIu64 ", but control file has %" PRIu64,
controlpath,
@@ -614,7 +615,7 @@ check_control_files(int n_backups, char **backup_dirs)
bool crc_ok;
char *controlpath;
- controlpath = psprintf("%s/%s", backup_dirs[i], "global/pg_control");
+ controlpath = psprintf("%s/%s", backup_dirs[i], XLOG_CONTROL_FILE);
pg_log_debug("reading \"%s\"", controlpath);
control_file = get_controlfile_by_exact_path(controlpath, &crc_ok);
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index 9901a2bae51..7bb801bb886 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -1,7 +1,7 @@
/*
* pg_controldata
*
- * reads the data from $PGDATA/global/pg_control
+ * reads the data from the control file located at $PGDATA/XLOG_CONTROL_FILE.
*
* copyright (c) Oliver Elphick <olly@lfix.co.uk>, 2001;
* license: BSD
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index a28d1667d4c..c933871ca9f 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -26,6 +26,7 @@
#include <sys/stat.h>
#include <unistd.h>
+#include "access/xlog_internal.h"
#include "catalog/pg_tablespace_d.h"
#include "common/file_utils.h"
#include "common/hashfn_unstable.h"
@@ -704,7 +705,7 @@ decide_file_action(file_entry_t *entry)
* Don't touch the control file. It is handled specially, after copying
* all the other files.
*/
- if (strcmp(path, "global/pg_control") == 0)
+ if (strcmp(path, XLOG_CONTROL_FILE) == 0)
return FILE_ACTION_NONE;
/* Skip macOS system files */
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 2c8b1a07007..9d16c1e6b47 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -328,7 +328,7 @@ main(int argc, char **argv)
* need to make sure by themselves that the target cluster is in a clean
* state.
*/
- buffer = slurpFile(datadir_target, "global/pg_control", &size);
+ buffer = slurpFile(datadir_target, XLOG_CONTROL_FILE, &size);
digestControlFile(&ControlFile_target, buffer, size);
pg_free(buffer);
@@ -338,12 +338,12 @@ main(int argc, char **argv)
{
ensureCleanShutdown(argv[0]);
- buffer = slurpFile(datadir_target, "global/pg_control", &size);
+ buffer = slurpFile(datadir_target, XLOG_CONTROL_FILE, &size);
digestControlFile(&ControlFile_target, buffer, size);
pg_free(buffer);
}
- buffer = source->fetch_file(source, "global/pg_control", &size);
+ buffer = source->fetch_file(source, XLOG_CONTROL_FILE, &size);
digestControlFile(&ControlFile_source, buffer, size);
pg_free(buffer);
@@ -636,7 +636,7 @@ perform_rewind(filemap_t *filemap, rewind_source *source,
* Fetch the control file from the source last. This ensures that the
* minRecoveryPoint is up-to-date.
*/
- buffer = source->fetch_file(source, "global/pg_control", &size);
+ buffer = source->fetch_file(source, XLOG_CONTROL_FILE, &size);
digestControlFile(&ControlFile_source_after, buffer, size);
pg_free(buffer);
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 47ee27ec835..8b7c349a875 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -12,6 +12,7 @@
#include <ctype.h>
#include <limits.h> /* for CHAR_MIN */
+#include "access/xlog_internal.h"
#include "common/string.h"
#include "pg_upgrade.h"
@@ -757,10 +758,10 @@ disable_old_cluster(transferMode transfer_mode)
new_path[MAXPGPATH];
/* rename pg_control so old server cannot be accidentally started */
- prep_status("Adding \".old\" suffix to old global/pg_control");
+ prep_status("Adding \".old\" suffix to old " XLOG_CONTROL_FILE);
- snprintf(old_path, sizeof(old_path), "%s/global/pg_control", old_cluster.pgdata);
- snprintf(new_path, sizeof(new_path), "%s/global/pg_control.old", old_cluster.pgdata);
+ snprintf(old_path, sizeof(old_path), "%s/%s", old_cluster.pgdata, XLOG_CONTROL_FILE);
+ snprintf(new_path, sizeof(new_path), "%s/%s.old", old_cluster.pgdata, XLOG_CONTROL_FILE);
if (pg_mv_file(old_path, new_path) != 0)
pg_fatal("could not rename file \"%s\" to \"%s\": %m",
old_path, new_path);
@@ -769,10 +770,10 @@ disable_old_cluster(transferMode transfer_mode)
if (transfer_mode == TRANSFER_MODE_LINK)
pg_log(PG_REPORT, "\n"
"If you want to start the old cluster, you will need to remove\n"
- "the \".old\" suffix from %s/global/pg_control.old.\n"
+ "the \".old\" suffix from %s/%s.old.\n"
"Because \"link\" mode was used, the old cluster cannot be safely\n"
"started once the new cluster has been started.",
- old_cluster.pgdata);
+ old_cluster.pgdata, XLOG_CONTROL_FILE);
else if (transfer_mode == TRANSFER_MODE_SWAP)
pg_log(PG_REPORT, "\n"
"Because \"swap\" mode was used, the old cluster can no longer be\n"
diff --git a/src/bin/pg_verifybackup/astreamer_verify.c b/src/bin/pg_verifybackup/astreamer_verify.c
index 079c3970410..268c5f0e246 100644
--- a/src/bin/pg_verifybackup/astreamer_verify.c
+++ b/src/bin/pg_verifybackup/astreamer_verify.c
@@ -14,6 +14,7 @@
#include "postgres_fe.h"
+#include "access/xlog_internal.h"
#include "catalog/pg_control.h"
#include "pg_verifybackup.h"
@@ -225,7 +226,7 @@ member_verify_header(astreamer *streamer, astreamer_member *member)
(!mystreamer->context->skip_checksums && should_verify_checksum(m));
mystreamer->verify_control_data =
mystreamer->context->manifest->version != 1 &&
- !m->bad && strcmp(m->pathname, "global/pg_control") == 0;
+ !m->bad && strcmp(m->pathname, XLOG_CONTROL_FILE) == 0;
/* If we're going to verify the checksum, initial a checksum context. */
if (mystreamer->verify_checksum &&
@@ -370,7 +371,7 @@ member_verify_control_data(astreamer *streamer)
pg_crc32c crc;
/* Should be here only for control file */
- Assert(strcmp(mystreamer->mfile->pathname, "global/pg_control") == 0);
+ Assert(strcmp(mystreamer->mfile->pathname, XLOG_CONTROL_FILE) == 0);
Assert(mystreamer->verify_control_data);
/*
diff --git a/src/bin/pg_verifybackup/pg_verifybackup.c b/src/bin/pg_verifybackup/pg_verifybackup.c
index 383668a8b6a..3ce233f3c20 100644
--- a/src/bin/pg_verifybackup/pg_verifybackup.c
+++ b/src/bin/pg_verifybackup/pg_verifybackup.c
@@ -18,6 +18,7 @@
#include <sys/stat.h>
#include <time.h>
+#include "access/xlog_internal.h"
#include "common/logging.h"
#include "common/parse_manifest.h"
#include "fe_utils/simple_list.h"
@@ -730,7 +731,7 @@ verify_plain_backup_file(verifier_context *context, char *relpath,
* version 1.
*/
if (context->manifest->version != 1 &&
- strcmp(relpath, "global/pg_control") == 0)
+ strcmp(relpath, XLOG_CONTROL_FILE) == 0)
verify_control_file(fullpath, context->manifest->system_identifier);
/* Update statistics for progress report, if necessary */
diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index 34d8a3a4e31..fa375dc9129 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -53,7 +53,7 @@ get_controlfile(const char *DataDir, bool *crc_ok_p)
{
char ControlFilePath[MAXPGPATH];
- snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
+ snprintf(ControlFilePath, MAXPGPATH, "%s/%s", DataDir, XLOG_CONTROL_FILE);
return get_controlfile_by_exact_path(ControlFilePath, crc_ok_p);
}
--
2.48.1
view thread (26+ messages) latest in thread
Message-ID: <18d80eb9-1e7a-4ead-98fe-60efc988954b@oss.nttdata.com>
Permalink: ../18d80eb9-1e7a-4ead-98fe-60efc988954b@oss.nttdata.com/
Also on: postgresql.org/message-id/18d80eb9-1e7a-4ead-98fe-60efc988954b@oss.nttdata.com
copy link · copy postgr.es · copy git am
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: masao.fujii@oss.nttdata.com, a.melnikov@postgrespro.ru, horikyota.ntt@gmail.com, daniel@yesql.se, peter@eisentraut.org, tgl@sss.pgh.pa.us, robertmhaas@gmail.com, michael@paquier.xyz
Subject: Re: Use XLOG_CONTROL_FILE macro everywhere?
In-Reply-To: <18d80eb9-1e7a-4ead-98fe-60efc988954b@oss.nttdata.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