Re: pg_verifybackup: TAR format backup verification

On Sat, Aug 17, 2024 at 1:34 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Fri, Aug 16, 2024 at 3:53 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > + int64 num = strtoi64(relpath, &suffix, 10);
>
> Hit send too early. Here, seems like this should be strtoul(), not strtoi64().
>

Fixed in the attached version including others suggestions in that mail.

> The documentation of --format seems to be cut-and-pasted from
> pg_basebackup and the language isn't really appropriate here. e.g.
> "The main data directory's contents will be written to a file
> named..." but pg_verifybackup writes nothing.
>

I wrote that intentionally -- I didn’t mean to imply that
pg_verifybackup handles this; rather, I meant that the backup tool (in
this case, pg_basebackup) produces those files. I can see the
confusion and have rephrased the text accordingly.

> + simple_string_list_append(&context.ignore_list, "pg_wal.tar");
> + simple_string_list_append(&context.ignore_list, "pg_wal.tar.gz");
> + simple_string_list_append(&context.ignore_list, "pg_wal.tar.lz4");
> + simple_string_list_append(&context.ignore_list, "pg_wal.tar.zst");
>
> Why not make the same logic that recognizes base or an OID also
> recognize pg_wal as a prefix, and identify that as the WAL archive?
> For now we'll have to skip it, but if you do it that way then if we
> add future support for more suffixes, it'll just work, whereas this
> way won't. And you'd need that code anyway if we ever can run
> pg_waldump on a tarfile, because you would need to identify the
> compression method. Note that the danger of the list of suffixes
> getting out of sync here is not hypothetical: you added .tgz elsewhere
> but not here.
>

Did this way.

> There's probably more to look at here but I'm running out of energy for today.
>

Thank you for the review and committing 0004 and 0006 patches.

Regards,
Amul