Re: Allow pg_archivecleanup to remove backup history files

torikoshia <torikoshia@oss.nttdata.com>

From: torikoshia <torikoshia@oss.nttdata.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-hackers@postgresql.org
Date: 2023-05-12T08:53:45Z
Lists: pgsql-hackers
On 2023-05-10 17:52, Bharath Rupireddy wrote:
Thanks for your comments!

> Just curious to know the driving point behind this proposal - is
> pg_archivecleanup deployed in production that was unable to clean up
> the history files and there were many such history files left? It will
> help us know how pg_archivecleanup is being used.

Yes.

> Just curious to know the driving point behind this proposal - is
> pg_archivecleanup deployed in production that was unable to clean up
> the history files and there were many such history files left? It will
> help us know how pg_archivecleanup is being used.
> 
> I'm wondering if making -x generic with '-x' '.backup', is simpler
> than adding another option?

Since according to the current semantics, deleting backup history files 
with -x demands not '-x .backup' but '-x .007C9330.backup' when the file 
name is 0000000100001234000055CD.007C9330.backup, it needs special 
treatment for backup history files, right?

I think it would be intuitive and easier to remember than new option.

I was a little concerned about what to do when deleting both the files 
ending in .gz and backup history files.
Is making it possible to specify both "-x .backup" and "-x .gz" the way 
to go?

I also concerned someone might add ".backup" to WAL files, but does that 
usually not happen?

> Comments on the patch:
> 1. Why just only the backup history files? Why not remove the timeline
> history files too? Is it because there may not be as many tli switches
> happening as backups?

Yeah, do you think we should also add logic for '-x .history'?

> 2.+sub remove_backuphistoryfile_run_check
> +{
> Why to invent a new function when run_check() can be made generic with
> few arguments passed?

Thanks, I'm going to reconsider it.

-- 
Regards,

--
Atsushi Torikoshi
NTT DATA CORPORATION



Commits

  1. pg_archivecleanup: Add --clean-backup-history

  2. pg_archivecleanup: Refactor loop doing old segment removals

  3. Introduce long options in pg_archivecleanup