From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: trying again to get incremental backup
Date: Fri, 17 Nov 2023 11:01:21 +0100
Message-ID: <202311171001.p5hwidowrrkj@alvherre.pgsql> (raw)
In-Reply-To: <CA+TgmobsXQV3n10=R5PGhwnMEjtGt7AFu9EuxCCf1MwXMGgRnQ@mail.gmail.com>
I made a pass over pg_combinebackup for NLS. I propose the attached
patch.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Right now the sectors on the hard disk run clockwise, but I heard a rumor that
you can squeeze 0.2% more throughput by running them counterclockwise.
It's worth the effort. Recommended." (Gerry Pourwelle)
Attachments:0001-do-NLS-for-pg_combinebackup.patch (text/x-diff, 7.2K) download
From a385542ff03514885fa4e84b0485e51cdcdd04bd Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 17 Nov 2023 10:51:36 +0100
Subject: [PATCH] do NLS for pg_combinebackup
---
src/bin/pg_combinebackup/backup_label.c | 34 +++++++++++----------
src/bin/pg_combinebackup/nls.mk | 11 +++++++
src/bin/pg_combinebackup/pg_combinebackup.c | 24 ++++++++++-----
src/bin/pg_combinebackup/reconstruct.c | 9 ++++--
4 files changed, 52 insertions(+), 26 deletions(-)
create mode 100644 src/bin/pg_combinebackup/nls.mk
diff --git a/src/bin/pg_combinebackup/backup_label.c b/src/bin/pg_combinebackup/backup_label.cindex 2a62aa6fad..922e00854d 100644--- a/src/bin/pg_combinebackup/backup_label.c+++ b/src/bin/pg_combinebackup/backup_label.c@@ -63,18 +63,18 @@ parse_backup_label(char *filename, StringInfo buf,
if (line_starts_with(s, e, "START WAL LOCATION: ", &s))
{
if (!parse_lsn(s, e, start_lsn, &c))
- pg_fatal("%s: could not parse START WAL LOCATION",- filename);+ pg_fatal("%s: could not parse %s",+ filename, "START WAL LOCATION");
if (c >= e || *c != ' ')
- pg_fatal("%s: improper terminator for START WAL LOCATION",- filename);+ pg_fatal("%s: improper terminator for %s",+ filename, "START WAL LOCATION");
found |= 1;
}
else if (line_starts_with(s, e, "START TIMELINE: ", &s))
{
if (!parse_tli(s, e, start_tli))
- pg_fatal("%s: could not parse TLI for START TIMELINE",- filename);+ pg_fatal("%s: could not parse TLI for %s",+ filename, "START TIMELINE");
if (*start_tli == 0)
pg_fatal("%s: invalid TLI", filename);
found |= 2;
@@ -82,18 +82,18 @@ parse_backup_label(char *filename, StringInfo buf,
else if (line_starts_with(s, e, "INCREMENTAL FROM LSN: ", &s))
{
if (!parse_lsn(s, e, previous_lsn, &c))
- pg_fatal("%s: could not parse INCREMENTAL FROM LSN",- filename);+ pg_fatal("%s: could not parse %s",+ filename, "INCREMENTAL FROM LSN");
if (c >= e || *c != '\n')
- pg_fatal("%s: improper terminator for INCREMENTAL FROM LSN",- filename);+ pg_fatal("%s: improper terminator for %s",+ filename, "INCREMENTAL FROM LSN");
found |= 4;
}
else if (line_starts_with(s, e, "INCREMENTAL FROM TLI: ", &s))
{
if (!parse_tli(s, e, previous_tli))
- pg_fatal("%s: could not parse INCREMENTAL FROM TLI",- filename);+ pg_fatal("%s: could not parse %s",+ filename, "INCREMENTAL FROM TLI");
if (*previous_tli == 0)
pg_fatal("%s: invalid TLI", filename);
found |= 8;
@@ -103,13 +103,15 @@ parse_backup_label(char *filename, StringInfo buf,
}
if ((found & 1) == 0)
- pg_fatal("%s: could not find START WAL LOCATION", filename);+ pg_fatal("%s: could not find %s", filename, "START WAL LOCATION");
if ((found & 2) == 0)
- pg_fatal("%s: could not find START TIMELINE", filename);+ pg_fatal("%s: could not find %s", filename, "START TIMELINE");
if ((found & 4) != 0 && (found & 8) == 0)
- pg_fatal("%s: INCREMENTAL FROM LSN requires INCREMENTAL FROM TLI", filename);+ pg_fatal("%s: %s requires %s", filename,+ "INCREMENTAL FROM LSN", "INCREMENTAL FROM TLI");
if ((found & 8) != 0 && (found & 4) == 0)
- pg_fatal("%s: INCREMENTAL FROM TLI requires INCREMENTAL FROM LSN", filename);+ pg_fatal("%s: %s requires %s", filename,+ "INCREMENTAL FROM TLI", "INCREMENTAL FROM LSN");
}
/*
diff --git a/src/bin/pg_combinebackup/nls.mk b/src/bin/pg_combinebackup/nls.mknew file mode 100644index 0000000000..c8e59d1d00--- /dev/null+++ b/src/bin/pg_combinebackup/nls.mk@@ -0,0 +1,11 @@+# src/bin/pg_combinebackup/nls.mk+CATALOG_NAME = pg_combinebackup+GETTEXT_FILES = $(FRONTEND_COMMON_GETTEXT_FILES) \+ backup_label.c \+ copy_file.c \+ load_manifest.c \+ pg_combinebackup.c \+ reconstruct.c \+ write_manifest.c+GETTEXT_TRIGGERS = $(FRONTEND_COMMON_GETTEXT_TRIGGERS)+GETTEXT_FLAGS = $(FRONTEND_COMMON_GETTEXT_FLAGS)diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.cindex 7bf56e57ae..618b5dd7f6 100644--- a/src/bin/pg_combinebackup/pg_combinebackup.c+++ b/src/bin/pg_combinebackup/pg_combinebackup.c@@ -521,29 +521,33 @@ check_control_files(int n_backups, char **backup_dirs)
{
ControlFileData *control_file;
bool crc_ok;
+ char *controlpath;- pg_log_debug("reading \"%s/global/pg_control\"", backup_dirs[i]);+ path = psprintf("%s/%s", backup_dirs[i], "global/pg_control");++ pg_log_debug("reading \"%s\"", controlpath);
control_file = get_controlfile(backup_dirs[i], &crc_ok);
/* Control file contents not meaningful if CRC is bad. */
if (!crc_ok)
- pg_fatal("%s/global/pg_control: crc is incorrect", backup_dirs[i]);+ pg_fatal("%s: crc is incorrect", controlpath);
/* Can't interpret control file if not current version. */
if (control_file->pg_control_version != PG_CONTROL_VERSION)
- pg_fatal("%s/global/pg_control: unexpected control file version",- backup_dirs[i]);+ pg_fatal("%s: unexpected control file version",+ controlpath);
/* System identifiers should all match. */
if (i == n_backups - 1)
system_identifier = control_file->system_identifier;
else if (system_identifier != control_file->system_identifier)
- pg_fatal("%s/global/pg_control: expected system identifier %llu, but found %llu",- backup_dirs[i], (unsigned long long) system_identifier,+ pg_fatal("%s: expected system identifier %llu, but found %llu",+ controlpath, (unsigned long long) system_identifier,
(unsigned long long) control_file->system_identifier);
/* Release memory. */
pfree(control_file);
+ pfree(controlpath);
}
/*
@@ -932,12 +936,16 @@ process_directory_recursively(Oid tsoid,
manifest_path);
if (mfile == NULL)
{
+ char *fullpath = psprintf("%s/%s", input_directory,+ backup_manifest);+
/*
* The directory is out of sync with the backup_manifest,
* so emit a warning.
*/
- pg_log_warning("\"%s/backup_manifest\" contains no entry for \"%s\"",- input_directory, manifest_path);+ pg_log_warning("\"%s\" contains no entry for \"%s\"",+ fullpath, manifest_path);+ pfree(fullpath);
}
else if (mfile->checksum_type == checksum_type)
{
diff --git a/src/bin/pg_combinebackup/reconstruct.c b/src/bin/pg_combinebackup/reconstruct.cindex e7f0523fe9..19ab96904b 100644--- a/src/bin/pg_combinebackup/reconstruct.c+++ b/src/bin/pg_combinebackup/reconstruct.c@@ -283,13 +283,18 @@ reconstruct_from_incremental_file(char *input_filename,
manifest_path);
if (mfile == NULL)
{
+ char *path = psprintf("%s/backup_manifest",+ prior_backup_dirs[copy_source_index]);+
/*
* The directory is out of sync with the backup_manifest, so emit
* a warning.
*/
- pg_log_warning("\"%s/backup_manifest\" contains no entry for \"%s\"",- prior_backup_dirs[copy_source_index],+ /*- translator: the first %s is a backup manifest file, the second is a file absent therein */+ pg_log_warning("\"%s\" contains no entry for \"%s\"",+ path,
manifest_path);
+ pfree(path);
}
else if (mfile->checksum_type == checksum_type)
{
--
2.39.2
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: alvherre@alvh.no-ip.org, robertmhaas@gmail.com, dilipbalaut@gmail.com, andres@anarazel.de
Subject: Re: trying again to get incremental backup
In-Reply-To: <202311171001.p5hwidowrrkj@alvherre.pgsql>
* 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