Re: pg_verifybackup: TAR format backup verification

Amul Sul <sulamul@gmail.com>

From: Amul Sul <sulamul@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Sravan Kumar <sravanvcybage@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2024-08-12T09:12:24Z
Lists: pgsql-hackers

Attachments

On Wed, Aug 7, 2024 at 11:28 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, Aug 7, 2024 at 1:05 PM Amul Sul <sulamul@gmail.com> wrote:
> > The main issue I have is computing the total_size of valid files that
> > will be checksummed and that exist in both the manifests and the
> > backup, in the case of a tar backup. This cannot be done in the same
> > way as with a plain backup.
>
> I think you should compute and sum the sizes of the tar files
> themselves. Suppose you readdir(), make a list of files that look
> relevant, and stat() each one. total_size is the sum of the file
> sizes. Then you work your way through the list of files and read each
> one. done_size is the total size of all files you've read completely
> plus the number of bytes you've read from the current file so far.
>

I tried this in the attached version and made a few additional changes
based on Sravan's off-list comments regarding function names and
descriptions.

Now, verification happens in two passes. The first pass simply
verifies the file names, determines their compression types, and
returns a list of valid tar files whose contents need to be verified
in the second pass. The second pass is called at the end of
verify_backup_directory() after all files in that directory have been
scanned. I named the functions for pass 1 and pass 2 as
verify_tar_file_name() and verify_tar_file_contents(), respectively.
The rest of the code flow is similar as in the previous version.

In the attached patch set, I abandoned the changes, touching the
progress reporting code of plain backups by dropping the previous 0009
patch. The new 0009 patch adds missing APIs to simple_list.c to
destroy SimplePtrList. The rest of the patch numbers remain unchanged.

Regards,
Amul

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.