Horton

Re: pg_verifybackup: TAR format backup verification

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Amul Sul <sulamul@gmail.com>
Cc: Sravan Kumar <sravanvcybage@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2024-08-16T20:04:35Z
Lists: pgsql-hackers 
On Fri, Aug 16, 2024 at 3:53 PM Robert Haas <robertmhaas@gmail.com> wrote:
> +        int64 num = strtoi64(relpath, &suffix, 10);

Hit send too early. Here, seems like this should be strtoul(), not strtoi64().

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.

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

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

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Commits

  1. Adjust json_manifest_per_file_callback API in one more place.

  2. File size in a backup manifest should use uint64, not size_t.

  3. Fix some pg_verifybackup issues reported by Coverity.

  4. pg_verifybackup: Move some declarations to new pg_verifybackup.h

  5. pg_verifybackup: Move skip_checksums into verifier_context.

  6. Improve file header comments for astramer code.

  7. Move astreamer (except astreamer_inject) to fe_utils.

  8. Move recovery injector astreamer to a separate header file.

  9. Rename bbstreamer to astreamer.