public inbox for [email protected]
help / color / mirror / Atom feedFrom: torikoshia <[email protected]>
To: Michael Paquier <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Subject: Re: Allow pg_archivecleanup to remove backup history files
Date: Tue, 09 May 2023 22:32:59 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <ZEhIiaCa2rdc/[email protected]>
References: <[email protected]>
<[email protected]>
<ZEhIiaCa2rdc/[email protected]>
Horiguchi-san, Michael-san
Thanks for your comments and information!
Attached a patch with documentation and regression tests.
On 2023-04-26 06:39, Michael Paquier wrote:
> On Tue, Apr 25, 2023 at 05:29:48PM +0900, Kyotaro Horiguchi wrote:
>> I thought that we have decided not to do that, but I coundn't find any
>> discussion about it in the ML archive. Anyway, I think it is great
>> that we have that option.
>
> No objections from here to make that optional. It's been argued for a
> couple of times that leaving the archive history files is good for
> debugging, but I can also get why one would one to clean up all the
> files older than a WAL retention policy. Even if these are just few
> bytes, it can be noisy for the eye to see a large accumulation of
> history files mixed with the WAL segments.
> --
> Michael
--
Regards,
--
Atsushi Torikoshi
NTT DATA CORPORATION
Attachments:
[text/x-diff] v2-0001-Allow-pg_archivecleanup-to-remove-backuphistoryfile.patch (5.9K, ../[email protected]/2-v2-0001-Allow-pg_archivecleanup-to-remove-backuphistoryfile.patch)
download | inline diff:
From 7d44134e5de930ce04819285a5b7359e370708d4 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <[email protected]>
Date: Tue, 9 May 2023 21:52:01 +0900
Subject: [PATCH v2] Allow pg_archivecleanup to remove backup history files
Add new option -b to remove files including backup history files older
than oldestkeptwalfile.
---
doc/src/sgml/ref/pgarchivecleanup.sgml | 11 +++++
src/bin/pg_archivecleanup/pg_archivecleanup.c | 11 ++++-
.../t/010_pg_archivecleanup.pl | 45 +++++++++++++++++--
3 files changed, 62 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/ref/pgarchivecleanup.sgml b/doc/src/sgml/ref/pgarchivecleanup.sgml
index 635e7c7685..6cb565156f 100644
--- a/doc/src/sgml/ref/pgarchivecleanup.sgml
+++ b/doc/src/sgml/ref/pgarchivecleanup.sgml
@@ -93,6 +93,17 @@ pg_archivecleanup: removing file "archive/00000001000000370000000E"
<variablelist>
+ <varlistentry>
+ <term><option>-b</option></term>
+ <listitem>
+ <para>
+ Remove files including backup history files, whose suffix is <filename>.backup</filename>.
+ Note that when <replaceable>oldestkeptwalfile</replaceable> is a backup history file,
+ specified file is kept and only preceding WAL files and backup history files are removed.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>-d</option></term>
<listitem>
diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c
index 7726d05149..ae6ae76f08 100644
--- a/src/bin/pg_archivecleanup/pg_archivecleanup.c
+++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c
@@ -23,6 +23,8 @@ const char *progname;
/* Options and defaults */
bool dryrun = false; /* are we performing a dry-run operation? */
+bool removeBackupHistoryFile = false; /* remove files including
+ * backup history files */
char *additional_ext = NULL; /* Extension to remove from filenames */
char *archiveLocation; /* where to find the archive? */
@@ -118,7 +120,8 @@ CleanupPriorWALFiles(void)
* file. Note that this means files are not removed in the order
* they were originally written, in case this worries you.
*/
- if ((IsXLogFileName(walfile) || IsPartialXLogFileName(walfile)) &&
+ if (((IsXLogFileName(walfile) || IsPartialXLogFileName(walfile)) ||
+ (removeBackupHistoryFile && IsBackupHistoryFileName(walfile))) &&
strcmp(walfile + 8, exclusiveCleanupFileName + 8) < 0)
{
char WALFilePath[MAXPGPATH * 2]; /* the file path
@@ -252,6 +255,7 @@ usage(void)
printf(_("Usage:\n"));
printf(_(" %s [OPTION]... ARCHIVELOCATION OLDESTKEPTWALFILE\n"), progname);
printf(_("\nOptions:\n"));
+ printf(_(" -b remove files including backup history files\n"));
printf(_(" -d generate debug output (verbose mode)\n"));
printf(_(" -n dry run, show the names of the files that would be removed\n"));
printf(_(" -V, --version output version information, then exit\n"));
@@ -294,10 +298,13 @@ main(int argc, char **argv)
}
}
- while ((c = getopt(argc, argv, "dnx:")) != -1)
+ while ((c = getopt(argc, argv, "bdnx:")) != -1)
{
switch (c)
{
+ case 'b': /* Remove backup history files too */
+ removeBackupHistoryFile = true;
+ break;
case 'd': /* Debug mode */
pg_logging_increase_verbosity();
break;
diff --git a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
index 76321d1284..afd2ff6209 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -16,9 +16,14 @@ my @walfiles = (
'00000001000000370000000C.gz', '00000001000000370000000D',
'00000001000000370000000E', '00000001000000370000000F.partial',);
+my @walfiles_with_backuphistoryfile = (
+ '000000020000003800000007.00000028.backup', '000000020000003800000008',
+ '000000020000003800000009',
+ '00000002000000380000000A', '00000002000000380000000B.007C9330.backup');
+
sub create_files
{
- foreach my $fn (@walfiles, 'unrelated_file')
+ foreach my $fn (@_, 'unrelated_file')
{
open my $file, '>', "$tempdir/$fn";
print $file 'CONTENT';
@@ -27,7 +32,7 @@ sub create_files
return;
}
-create_files();
+create_files(@walfiles);
command_fails_like(
['pg_archivecleanup'],
@@ -76,7 +81,7 @@ sub run_check
my ($suffix, $test_name) = @_;
- create_files();
+ create_files(@walfiles);
command_ok(
[
@@ -98,8 +103,42 @@ sub run_check
return;
}
+sub remove_backuphistoryfile_run_check
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my ($suffix, $test_name) = @_;
+
+ create_files(@walfiles_with_backuphistoryfile);
+
+ command_ok(
+ [
+ 'pg_archivecleanup', '-b', $tempdir,
+ $walfiles_with_backuphistoryfile[2] . $suffix
+ ],
+ "$test_name: remove_backuphistoryfile_runs");
+
+ ok(!-f "$tempdir/$walfiles_with_backuphistoryfile[0]",
+ "$test_name: first .backup file was cleaned up");
+ ok(!-f "$tempdir/$walfiles_with_backuphistoryfile[1]",
+ "$test_name: second older WAL file was cleaned up");
+ ok(-f "$tempdir/$walfiles_with_backuphistoryfile[2]",
+ "$test_name: restartfile was not cleaned up");
+ ok(-f "$tempdir/$walfiles_with_backuphistoryfile[3]",
+ "$test_name: newer WAL file was not cleaned up");
+ ok(-f "$tempdir/$walfiles_with_backuphistoryfile[4]",
+ "$test_name: newer .backup file was not cleaned up");
+ ok(-f "$tempdir/unrelated_file",
+ "$test_name: unrelated file was not cleaned up");
+ return;
+}
+
run_check('', 'pg_archivecleanup');
run_check('.partial', 'pg_archivecleanup with .partial file');
run_check('.00000020.backup', 'pg_archivecleanup with .backup file');
+remove_backuphistoryfile_run_check('', 'pg_archivecleanup -b');
+remove_backuphistoryfile_run_check('.00000020.backup',
+ 'pg_archivecleanup -b with .backup file');
+
done_testing();
base-commit: c5b7f67fcc8c4a01c82660eb0996a3c697fac283
--
2.25.1
view thread (6+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: Allow pg_archivecleanup to remove backup history files
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox