v4-0002-Allow-pg_archivecleanup-to-remove-backup-history-.patch

From 23d20f30eb0fa6daed1f5e631c5b3bedaeae623d Mon Sep 17 00:00:00 2001
From: Atsushi Torikoshi <torikoshia@oss.nttdata.com>
Date: Thu, 25 May 2023 23:25:28 +0900
Subject: [PATCH v4 2/2] 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 | 78 ++++++++++++-------
 .../t/010_pg_archivecleanup.pl                | 46 ++++++-----
 3 files changed, 87 insertions(+), 48 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 707d7d54cf..3fd4a441b2 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? */
@@ -97,6 +99,8 @@ CleanupPriorWALFiles(void)
 	{
 		while (errno = 0, (xlde = readdir(xldir)) != NULL)
 		{
+			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
@@ -106,6 +110,19 @@ CleanupPriorWALFiles(void)
 			TrimExtension(walfile, additional_ext);
 
 			/*
+			 * Check file name.
+			 *
+			 * We skip files which are not WAL file or partial WAL file.
+			 * Also we skip backup history files when --clean-backup-history
+			 * is not specified.
+			 */
+			if (!IsXLogFileName(walfile) && !IsPartialXLogFileName(walfile) &&
+				(!cleanBackupHistory || !IsBackupHistoryFileName(walfile)))
+				continue;
+
+			/*
+			 * Check cutoff point.
+			 *
 			 * We ignore the timeline part of the XLOG segment identifiers in
 			 * deciding whether a segment is still needed.  This ensures that
 			 * we won't prematurely remove a segment from a parent timeline.
@@ -118,39 +135,35 @@ 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)) &&
-				strcmp(walfile + 8, exclusiveCleanupFileName + 8) < 0)
-			{
-				char		WALFilePath[MAXPGPATH * 2]; /* the file path
-														 * including archive */
+			if (strcmp(walfile + 8, exclusiveCleanupFileName + 8) >= 0)
+				continue;
 
+			/*
+			 * Use the original file name again now, including any
+			 * extension that might have been chopped off before testing
+			 * the sequence.
+			 */
+			snprintf(WALFilePath, sizeof(WALFilePath), "%s/%s",
+					 archiveLocation, xlde->d_name);
+
+			if (dryrun)
+			{
 				/*
-				 * Use the original file name again now, including any
-				 * extension that might have been chopped off before testing
-				 * the sequence.
+				 * Prints the name of the file to be removed and skips the
+				 * actual removal.  The regular printout is so that the
+				 * user can pipe the output into some other program.
 				 */
-				snprintf(WALFilePath, sizeof(WALFilePath), "%s/%s",
-						 archiveLocation, xlde->d_name);
-
-				if (dryrun)
-				{
-					/*
-					 * Prints the name of the file to be removed and skips the
-					 * actual removal.  The regular printout is so that the
-					 * user can pipe the output into some other program.
-					 */
-					printf("%s\n", WALFilePath);
-					pg_log_debug("file \"%s\" would be removed", WALFilePath);
-					continue;
-				}
-
-				pg_log_debug("removing file \"%s\"", WALFilePath);
-
-				rc = unlink(WALFilePath);
-				if (rc != 0)
-					pg_fatal("could not remove file \"%s\": %m",
-							 WALFilePath);
+				printf("%s\n", WALFilePath);
+				pg_log_debug("file \"%s\" would be removed", WALFilePath);
+				continue;
 			}
+
+			pg_log_debug("removing file \"%s\"", WALFilePath);
+
+			rc = unlink(WALFilePath);
+			if (rc != 0)
+				pg_fatal("could not remove file \"%s\": %m",
+						 WALFilePath);
 		}
 
 		if (errno)
@@ -254,6 +267,7 @@ usage(void)
 	printf(_("\nOptions:\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 removed\n"));
+	printf(_("  -b, --clean-backup-history  clean up files including backup history files\n"));
 	printf(_("  -V, --version               output version information, then exit\n"));
 	printf(_("  -x --strip-extension=EXT    clean up files if they have this extension\n"));
 	printf(_("  -?, --help                  show this help, then exit\n"));
@@ -275,6 +289,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'},
@@ -300,10 +315,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..ee2f714c2a 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -12,13 +12,18 @@ program_options_handling_ok('pg_archivecleanup');
 
 my $tempdir = PostgreSQL::Test::Utils::tempdir;
 
-my @walfiles = (
+my @walfiles_with_gz = (
 	'00000001000000370000000C.gz', '00000001000000370000000D',
 	'00000001000000370000000E', '00000001000000370000000F.partial',);
 
+my @walfiles_with_backup = (
+	'00000001000000370000000D',
+	'00000001000000370000000D.00000028.backup',
+	'00000001000000370000000E',    '00000001000000370000000F.partial',);
+
 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_with_gz);
 
 command_fails_like(
 	['pg_archivecleanup'],
@@ -59,14 +64,14 @@ command_fails_like(
 	my $stderr;
 	my $result =
 	  IPC::Run::run [ 'pg_archivecleanup', '-d', '-n', $tempdir,
-		$walfiles[2] ],
+		$walfiles_with_gz[2] ],
 	  '2>', \$stderr;
 	ok($result, "pg_archivecleanup dry run: exit code 0");
 	like(
 		$stderr,
-		qr/$walfiles[1].*would be removed/,
+		qr/$walfiles_with_gz[1].*would be removed/,
 		"pg_archivecleanup dry run: matches");
-	foreach my $fn (@walfiles)
+	foreach my $fn (@walfiles_with_gz)
 	{
 		ok(-f "$tempdir/$fn", "$fn not removed");
 	}
@@ -76,32 +81,37 @@ sub run_check
 {
 	local $Test::Builder::Level = $Test::Builder::Level + 1;
 
-	my ($suffix, $test_name) = @_;
+	my ($walfiles, $suffix, $test_name, @options) = @_;
 
-	create_files();
+	create_files(\@$walfiles);
 
 	command_ok(
 		[
-			'pg_archivecleanup', '-x', '.gz', $tempdir,
-			$walfiles[2] . $suffix
+			'pg_archivecleanup', @options, $tempdir,
+			@$walfiles[2] . $suffix
 		],
 		"$test_name: runs");
 
-	ok(!-f "$tempdir/$walfiles[0]",
+	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]",
+	ok(!-f "$tempdir/@$walfiles[1]",
+		"$test_name: second older WAL/backup history file was cleaned up");
+	ok(-f "$tempdir/@$walfiles[2]",
 		"$test_name: restartfile was not cleaned up");
-	ok(-f "$tempdir/$walfiles[3]",
+	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");
 	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, '',
+	'pg_archivecleanup', '-x.gz');
+run_check(\@walfiles_with_gz, '.partial',
+	'pg_archivecleanup with .partial file', '-x.gz');
+run_check(\@walfiles_with_gz, '.00000020.backup',
+	'pg_archivecleanup with .backup file', '-x.gz');
+run_check(\@walfiles_with_backup, '',
+	'pg_archivecleanup with --clean-backup-history', '-b');
 
 done_testing();
-- 
2.39.2