Re: Allow pg_archivecleanup to remove backup history files

At Thu, 25 May 2023 23:51:18 +0900, torikoshia <torikoshia@oss.nttdata.com> wrote in 
> Updated patches according to your comment.

+    printf(_("  -x --strip-extension=EXT    clean up files if they have this extension\n"));

Perhaps a comma is needed after "-x". The apparent inconsistency
between the long name and the description is perplexing. I think it
might be more suitable to reword the descrition to "ignore this
extension while identifying files for cleanup" or something along
those lines..., and then name the option as "--ignore-extension".


The patch leaves the code.

>             * 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.)
>             */
>            strlcpy(walfile, xlde->d_name, MAXPGPATH);
>            TrimExtension(walfile, additional_ext);

The comment is no longer correct about the file name length.



+            if (!IsXLogFileName(walfile) && !IsPartialXLogFileName(walfile) &&
+                (!cleanBackupHistory || !IsBackupHistoryFileName(walfile)))
+                continue;

I'm not certain about the others, but I see this a tad tricky to
understand at first glance. Here's how I would phrase it. (A nearby
comment might require a tweak if we go ahead with this change.)

        if (!(IsXLogFileName(walfile) || IsPartialXLogFileName(walfile) ||
            (cleanBackupHistory && IsBackupHistoryFileName(walfile))))

or 

        if (!IsXLogFileName(walfile) && !IsPartialXLogFileName(walfile) &&
            !(cleanBackupHistory && IsBackupHistoryFileName(walfile)))


By the way, this patch reduces the nesting level. If we go that
direction, the following structure could be reworked similarly, and I
believe it results in a more standard structure.

    if ((xldir = opendir(archiveLocation)) != NULL)
    {
    ...
    }
    else
      pg_fatal("could not open archive location..

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center