From: | Amul Sul <sulamul(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Sravan Kumar <sravanvcybage(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pg_verifybackup: TAR format backup verification |
Date: | 2024-08-20 10:26:23 |
Message-ID: | CAAJ_b95e9oNpQ+EdLgKX+aLJXwpWAy2EZsLBFSB9=XSPs4hhFg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
v11-0012-pg_verifybackup-Tests-and-document.patch | application/x-patch | 12.6 KB |
v11-0011-pg_verifybackup-Read-tar-files-and-verify-its-co.patch | application/x-patch | 27.7 KB |
v11-0010-pg_verifybackup-Add-backup-format-and-compressio.patch | application/x-patch | 6.1 KB |
v11-0009-Add-simple_ptr_list_destroy-and-simple_ptr_list_.patch | application/x-patch | 2.2 KB |
v11-0008-Refactor-split-verify_control_file.patch | application/x-patch | 5.4 KB |
v11-0007-Refactor-split-verify_file_checksum-function.patch | application/octet-stream | 3.2 KB |
v11-0006-Refactor-split-verify_backup_file-function-and-r.patch | application/octet-stream | 6.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Eisentraut | 2024-08-20 10:38:25 | Re: Virtual generated columns |
Previous Message | Thomas Munro | 2024-08-20 10:09:27 | Re: Some questions about PostgreSQL’s design. |