Re: Allow pg_archivecleanup to remove backup history files

On Mon, May 22, 2023 at 06:24:49PM +0900, torikoshia wrote:
> Thanks for your advice, attached patches.

0001 looks OK, thanks!

+ Remove files including backup history file.

This could be reworded as "Remove backup history files.", I assume.

+ 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.

The same thing is described at the bottom of the documentation, so it
does not seem necessary here.

- 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(_(" -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"));
+ 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"));

Perhaps this indentation had better be adjusted in 0001, no big deal
either way.

- ok(!-f "$tempdir/$walfiles[0]",
- "$test_name: first older WAL file was cleaned up");
+ if (grep {$_ eq '-x.gz'} @options) {
+ ok(!-f "$tempdir/$walfiles[0]",
+ "$test_name: first older WAL file with .gz was cleaned up");
+ } else {
+ ok(-f "$tempdir/$walfiles[0]",
+ "$test_name: first older WAL file with .gz was not cleaned up");
+ }
[...]
- ok(-f "$tempdir/$walfiles[2]",
- "$test_name: restartfile was not cleaned up");
+ if (grep {$_ eq '-b'} @options) {
+ ok(!-f "$tempdir/$walfiles[2]",
+ "$test_name: Backup history file was cleaned up");
+ } else {
+ ok(-f "$tempdir/$walfiles[2]",
+ "$test_name: Backup history file was not cleaned up");
+ }

That's over-complicated, caused by the fact that all the tests rely on
the same list of WAL files to create, aka @walfiles defined at the
beginning of the script. Wouldn't it be cleaner to handle the fake
file creations with more than one global list, before each command
run? The list of files to be created could just be passed down as an
argument of run_check(), for example, before cleaning up the contents
for the next command.
--
Michael