From 110aeb49d8fb0c50777e719ae75fd3faa4b759c3 Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Date: Wed, 21 Jun 2023 11:36:28 +0900
Subject: [PATCH v10 3/3] Allow pg_archivecleanup to remove backup history
 files

Backup history files are just few bytes, but it can be noisy for the
eye to see a large accumulation of history files mixed with the WAL
segments.
This patch adds a new option to remove files including backup
history files older oldestkeptwalfile.
---
 doc/src/sgml/ref/pgarchivecleanup.sgml        | 11 +++
 src/bin/pg_archivecleanup/pg_archivecleanup.c | 19 +++--
 .../t/010_pg_archivecleanup.pl                | 79 +++++++++++++------
 3 files changed, 79 insertions(+), 30 deletions(-)

diff --git a/doc/src/sgml/ref/pgarchivecleanup.sgml b/doc/src/sgml/ref/pgarchivecleanup.sgml
index 09991c2fcd..8fac233db6 100644
--- a/doc/src/sgml/ref/pgarchivecleanup.sgml
+++ b/doc/src/sgml/ref/pgarchivecleanup.sgml
@@ -113,6 +113,17 @@ pg_archivecleanup:  removing file "archive/00000001000000370000000E"
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>-b</option></term>
+      <term><option>--clean-backup-history</option></term>
+      <listitem>
+       <para>
+         Remove backup history files.
+         For details about backup history file, please refer to the <xref linkend="backup-base-backup"/>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option>-V</option></term>
       <term><option>--version</option></term>
diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c
index f7269068cb..ffb4a19e7f 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		cleanBackupHistory = false;	/* remove files including
+												 * backup history files */
 char	   *additional_ext = NULL;	/* Extension to remove from filenames */
 
 char	   *archiveLocation;	/* where to find the archive? */
@@ -102,9 +104,10 @@ CleanupPriorWALFiles(void)
 		char		WALFilePath[MAXPGPATH * 2]; /* the file path
 													 * including archive */
 		/*
-		 * Truncation is essentially harmless, because we skip names of
-		 * length other than XLOG_FNAME_LEN.  (In principle, one could use
-		 * a 1000-character additional_ext and get trouble.)
+		 * Truncation is essentially harmless,  because we check the file
+		 * format including the length immediately after this.
+		 * (In principle, one could use a 1000-character additional_ext
+		 * and get trouble.)
 		 */
 		strlcpy(walfile, xlde->d_name, MAXPGPATH);
 		TrimExtension(walfile, additional_ext);
@@ -116,7 +119,8 @@ CleanupPriorWALFiles(void)
 		 * Also we skip backup history files when --clean-backup-history
 		 * is not specified.
 		 */
-		if (!IsXLogFileName(walfile) && !IsPartialXLogFileName(walfile))
+		if (!IsXLogFileName(walfile) && !IsPartialXLogFileName(walfile) &&
+			!(cleanBackupHistory && IsBackupHistoryFileName(walfile)))
 			continue;
 
 		/*
@@ -260,6 +264,7 @@ usage(void)
 	printf(_("Usage:\n"));
 	printf(_("  %s [OPTION]... ARCHIVELOCATION OLDESTKEPTWALFILE\n"), progname);
 	printf(_("\nOptions:\n"));
+	printf(_("  -b, --clean-backup-history  clean up files including backup history files\n"));
 	printf(_("  -d, --debug                 generate debug output (verbose mode)\n"));
 	printf(_("  -n, --dry-run               dry run, show the names of the files that would be\n"
 			 "                              removed\n"));
@@ -285,6 +290,7 @@ int
 main(int argc, char **argv)
 {
 	static struct option long_options[] = {
+		{"clean-backup-history", no_argument, NULL, 'b'},
 		{"debug", no_argument, NULL, 'd'},
 		{"dry-run", no_argument, NULL, 'n'},
 		{"strip-extension", required_argument, NULL, 'x'},
@@ -310,10 +316,13 @@ main(int argc, char **argv)
 		}
 	}
 
-	while ((c = getopt_long(argc, argv, "dnx:", long_options, NULL)) != -1)
+	while ((c = getopt_long(argc, argv, "bdnx:", long_options, NULL)) != -1)
 	{
 		switch (c)
 		{
+			case 'b': 			/* Remove backup history files too */
+				cleanBackupHistory = 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 cc3386d146..030a82e7fa 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -12,13 +12,23 @@ program_options_handling_ok('pg_archivecleanup');
 
 my $tempdir = PostgreSQL::Test::Utils::tempdir;
 
-my @walfiles = (
-	'00000001000000370000000C.gz', '00000001000000370000000D',
-	'00000001000000370000000E', '00000001000000370000000F.partial',);
+my @walfiles_with_gz = (
+	{ name => '00000001000000370000000C.gz',      present => 0 },
+	{ name => '00000001000000370000000D',         present => 0 },
+	{ name => '00000001000000370000000E',         present => 1 },
+	{ name => '00000001000000370000000F.partial', present => 1 },
+	{ name => 'unrelated_file',                   present => 1 });
+
+my @walfiles_with_backup = (
+	{ name => '00000001000000370000000D',                 present => 0 },
+	{ name => '00000001000000370000000D.00000028.backup', present => 0 },
+	{ name => '00000001000000370000000E',                 present => 1 },
+	{ name => '00000001000000370000000F.partial',         present => 1 },
+	{ name => 'unrelated_file',                           present => 1 });
 
 sub create_files
 {
-	foreach my $fn (@walfiles, 'unrelated_file')
+	foreach my $fn (@_)
 	{
 		open my $file, '>', "$tempdir/$fn";
 		print $file 'CONTENT';
@@ -27,7 +37,18 @@ sub create_files
 	return;
 }
 
-create_files();
+sub get_walfiles
+{
+	my @walfiles;
+
+	for my $walpair (@_)
+	{
+		push @walfiles, $walpair->{name};
+	}
+	return @walfiles;
+}
+
+create_files(get_walfiles(@walfiles_with_gz));
 
 command_fails_like(
 	['pg_archivecleanup'],
@@ -59,14 +80,14 @@ command_fails_like(
 	my $stderr;
 	my $result =
 	  IPC::Run::run [ 'pg_archivecleanup', '-d', '-n', $tempdir,
-		$walfiles[2] ],
+		$walfiles_with_gz[2]->{name} ],
 	  '2>', \$stderr;
 	ok($result, "pg_archivecleanup dry run: exit code 0");
 	like(
 		$stderr,
-		qr/$walfiles[1].*would be removed/,
+		qr/$walfiles_with_gz[1]->{name}.*would be removed/,
 		"pg_archivecleanup dry run: matches");
-	foreach my $fn (@walfiles)
+	foreach my $fn (get_walfiles(@walfiles_with_gz))
 	{
 		ok(-f "$tempdir/$fn", "$fn not removed");
 	}
@@ -76,32 +97,40 @@ sub run_check
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 
-	my ($suffix, $test_name) = @_;
+	my ($testdata, $oldestkeptwalfile, $test_name, @options) = @_;
 
-	create_files();
+	create_files(get_walfiles(@$testdata));
 
 	command_ok(
 		[
-			'pg_archivecleanup', '-x', '.gz', $tempdir,
-			$walfiles[2] . $suffix
+			'pg_archivecleanup', @options, $tempdir,
+			$oldestkeptwalfile
 		],
 		"$test_name: runs");
 
-	ok(!-f "$tempdir/$walfiles[0]",
-		"$test_name: first older WAL file was cleaned up");
-	ok(!-f "$tempdir/$walfiles[1]",
-		"$test_name: second older WAL file was cleaned up");
-	ok(-f "$tempdir/$walfiles[2]",
-		"$test_name: restartfile was not cleaned up");
-	ok(-f "$tempdir/$walfiles[3]",
-		"$test_name: newer WAL file was not cleaned up");
-	ok(-f "$tempdir/unrelated_file",
-		"$test_name: unrelated file was not cleaned up");
+	for my $walpair (@$testdata)
+	{
+		if ($walpair->{present})
+		{
+			ok(-f "$tempdir/$walpair->{name}",
+				"$test_name:$walpair->{name} was not cleaned up");
+		}
+		else
+		{
+			ok(!-f "$tempdir/$walpair->{name}",
+				"$test_name:$walpair->{name} was cleaned up");
+		}
+	}
 	return;
 }
 
-run_check('', 'pg_archivecleanup');
-run_check('.partial', 'pg_archivecleanup with .partial file');
-run_check('.00000020.backup', 'pg_archivecleanup with .backup file');
+run_check(\@walfiles_with_gz, '00000001000000370000000E',
+	'pg_archivecleanup', '-x.gz');
+run_check(\@walfiles_with_gz, '00000001000000370000000E.partial',
+	'pg_archivecleanup with .partial file', '-x.gz');
+run_check(\@walfiles_with_gz, '00000001000000370000000E.00000020.backup',
+	'pg_archivecleanup with .backup file', '-x.gz');
+run_check(\@walfiles_with_backup, '00000001000000370000000E',
+	'pg_archivecleanup with --clean-backup-history', '-b');
 
 done_testing();
-- 
2.39.3

